-
Notifications
You must be signed in to change notification settings - Fork 11
Refactor OwnerStrategy to RevisionMetadata #427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The primary purpose of this refactor is to remove the hard-coded association between a Revision and a client.Object. It allows the creation of a RevisionMetadata implementation which does not require a separate kubernetes object to exist for each Revision.
WalkthroughThis pull request refactors the ownership handling system from an owner-based strategy pattern to a metadata-based abstraction. New Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@boxcutter.go`:
- Around line 141-150: The code unconditionally constructs a new PhaseValidator
via validation.NewPhaseValidator, ignoring any custom PhaseValidator provided on
RevisionEngineOptions; update the pval initialization to use the supplied
opts.RevisionEngineOptions.PhaseValidator when non-nil and only call
validation.NewPhaseValidator(opts.RestMapper, opts.Writer) as a fallback so
RevisionEngine honors injected validators (adjust the pval assignment near the
NewPhaseValidator call and ensure the variable name pval is used by
machinery.NewComparator/machinery.NewObjectEngine as before).
In `@ownerhandling/annotation_test.go`:
- Around line 83-85: The test uses require.Error(t, err) then calls
assert.ErrorAs(t, err, &alreadyOwnedErr) which can continue on failure; replace
the assertion with require.ErrorAs(t, err, &alreadyOwnedErr) so the test fails
fast and avoids nil-pointer issues—update the call to require.ErrorAs
referencing the same variable alreadyOwnedErr and error value err in the
ownerhandling/annotation_test.go test.
In `@ownerhandling/annotation.go`:
- Around line 166-179: GetCurrent currently converts an annotationOwnerRef to
metav1.OwnerReference via ToMetaV1OwnerRef which drops Namespace; instead
preserve namespace by returning a RevisionReference that carries namespace
information — either return the original annotationOwnerRef (from
getOwnerReferences) or construct/return a wrapper implementing
bctypes.RevisionReference that embeds the metav1.OwnerReference plus a Namespace
field. Update GetCurrent in annotationRevisionMetadata and, if needed, adjust
ToMetaV1OwnerRef or add a new conversion method so the returned object from
GetCurrent still exposes Namespace (refer to GetCurrent, getOwnerReferences,
annotationOwnerRef and ToMetaV1OwnerRef).
In `@ownerhandling/native.go`:
- Around line 23-36: The constructor NewNativeRevisionMetadata currently ignores
the allowCrossNamespace parameter and IsNamespaceAllowed always enforces
same-namespace; store allowCrossNamespace on the NativeRevisionMetadata struct
(add a field like allowCrossNamespace bool), set it in
NewNativeRevisionMetadata, and update the IsNamespaceAllowed method to return
true when allowCrossNamespace is true (or when the owner is cluster-scoped)
otherwise enforce that target namespace equals owner.GetNamespace(); reference
NativeRevisionMetadata, NewNativeRevisionMetadata, and IsNamespaceAllowed when
making these changes.
- Around line 103-114: The CopyReferences implementation on
NativeRevisionMetadata is mutating the same OwnerReferences slice returned from
objA by calling objB.SetOwnerReferences(objA.GetOwnerReferences()) then changing
ownerRefs[i].Controller, which can mutate objA; instead, create a deep copy of
the owner references before mutating: obtain refs := objA.GetOwnerReferences(),
allocate a new slice of the same length, copy each metav1.OwnerReference into
the new slice (or append a struct literal per element) and set Controller =
ptr.To(false) on the copied elements, then call objB.SetOwnerReferences with
that new slice so objA is never modified.
In `@test/revision_engine_basic_test.go`:
- Around line 55-59: NewNativeRevisionMetadata is being called before the owner
UID exists which causes a panic when UID is empty; move creation of
revOwnerMetadata and the boxcutter.Revision rev until after you create the owner
(i.e., after calling Client.Create on revOwner) or re-fetch the owner to obtain
its UID, then construct revOwnerMetadata with
ownerhandling.NewNativeRevisionMetadata and build rev using that metadata so UID
is non-empty.
🧹 Nitpick comments (5)
cmd/reference/internal/util.go (1)
34-37: Type assertion without safe check could cause runtime panics.The unchecked type assertion on line 36 will panic if
revision.Metadatais not a*NativeRevisionMetadata. While the comment acknowledges this, consider using a safe type assertion with anokcheck to provide a more informative error message, especially since this is called in a loop inprevJSON.💡 Suggested improvement
func getNativeOwner(revision boxcutter.Revision) client.Object { - // NOTE: Will panic if not a NativeRevisionMetadata - return revision.Metadata.(*ownerhandling.NativeRevisionMetadata).GetOwner() + // NOTE: Will panic if not a NativeRevisionMetadata + nativeMeta, ok := revision.Metadata.(*ownerhandling.NativeRevisionMetadata) + if !ok { + panic(fmt.Sprintf("expected *NativeRevisionMetadata, got %T", revision.Metadata)) + } + return nativeMeta.GetOwner() }ownerhandling/annotation.go (1)
29-31: Add documentation comment for exported function.Per static analysis,
NewAnnotationHandleris exported but lacks a documentation comment. This is a public API that should be documented.📝 Suggested fix
+// NewAnnotationHandler creates a handler for annotation-based revision metadata operations. +// The annotationKey specifies which annotation to use for storing owner references. func NewAnnotationHandler(annotationKey string) annotationHandler { return annotationHandler{annotationKey: annotationKey} }ownerhandling/annotation_test.go (1)
659-705: Duplicate test:TestAnnotationOwnerRef_IsControllerduplicatesTestIsController.This test (lines 659-705) is essentially identical to
TestIsController(lines 266-312). Both test theisController()method with the same scenarios. Consider removing one to avoid maintenance burden.validation/object.go (1)
106-141: Remove unusedvalidateNamespacefunction.The
validateNamespacefunction is no longer used in production code and has been replaced bymetadata.IsNamespaceAllowed(obj)in theValidatemethod (line 57). It only appears in test code (validation/object_test.go:370). Removing this function and its corresponding test will reduce code maintenance burden.cmd/reference/internal/deploy.go (1)
403-425: Consider pre-allocating thepreviousMetadataslice.Since the length of
previousUnstris known, pre-allocatingpreviousMetadataavoids repeated slice growth allocations, as flagged by static analysis.♻️ Suggested fix
- var previousMetadata []boxcutter.RevisionMetadata - for i := range previousUnstr { + previousMetadata := make([]boxcutter.RevisionMetadata, 0, len(previousUnstr)) + + for i := range previousUnstr { previous = append(previous, &previousUnstr[i]) previousMetadata = append(previousMetadata, ownerhandling.NewNativeRevisionMetadata(&previousUnstr[i], c.scheme, false)) }
| pval := validation.NewPhaseValidator(opts.RestMapper, opts.Writer) | ||
| rval := validation.NewRevisionValidator() | ||
|
|
||
| comp := machinery.NewComparator( | ||
| opts.OwnerStrategy, opts.DiscoveryClient, opts.Scheme, opts.FieldOwner) | ||
| opts.DiscoveryClient, opts.Scheme, opts.FieldOwner) | ||
|
|
||
| oe := machinery.NewObjectEngine( | ||
| opts.Scheme, opts.Reader, opts.Writer, | ||
| opts.OwnerStrategy, comp, opts.FieldOwner, opts.SystemPrefix, | ||
| comp, opts.FieldOwner, opts.SystemPrefix, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Respect RevisionEngineOptions.PhaseValidator when provided.
Line 141 always constructs a new PhaseValidator, which ignores a custom validator supplied in RevisionEngineOptions. This breaks configurability and differs from NewPhaseEngine.
🔧 Suggested fix
- pval := validation.NewPhaseValidator(opts.RestMapper, opts.Writer)
+ pval := opts.PhaseValidator
+ if pval == nil {
+ pval = validation.NewPhaseValidator(opts.RestMapper, opts.Writer)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pval := validation.NewPhaseValidator(opts.RestMapper, opts.Writer) | |
| rval := validation.NewRevisionValidator() | |
| comp := machinery.NewComparator( | |
| opts.OwnerStrategy, opts.DiscoveryClient, opts.Scheme, opts.FieldOwner) | |
| opts.DiscoveryClient, opts.Scheme, opts.FieldOwner) | |
| oe := machinery.NewObjectEngine( | |
| opts.Scheme, opts.Reader, opts.Writer, | |
| opts.OwnerStrategy, comp, opts.FieldOwner, opts.SystemPrefix, | |
| comp, opts.FieldOwner, opts.SystemPrefix, | |
| ) | |
| pval := opts.PhaseValidator | |
| if pval == nil { | |
| pval = validation.NewPhaseValidator(opts.RestMapper, opts.Writer) | |
| } | |
| rval := validation.NewRevisionValidator() | |
| comp := machinery.NewComparator( | |
| opts.DiscoveryClient, opts.Scheme, opts.FieldOwner) | |
| oe := machinery.NewObjectEngine( | |
| opts.Scheme, opts.Reader, opts.Writer, | |
| comp, opts.FieldOwner, opts.SystemPrefix, | |
| ) |
🤖 Prompt for AI Agents
In `@boxcutter.go` around lines 141 - 150, The code unconditionally constructs a
new PhaseValidator via validation.NewPhaseValidator, ignoring any custom
PhaseValidator provided on RevisionEngineOptions; update the pval initialization
to use the supplied opts.RevisionEngineOptions.PhaseValidator when non-nil and
only call validation.NewPhaseValidator(opts.RestMapper, opts.Writer) as a
fallback so RevisionEngine honors injected validators (adjust the pval
assignment near the NewPhaseValidator call and ensure the variable name pval is
used by machinery.NewComparator/machinery.NewObjectEngine as before).
| require.Error(t, err) | ||
| var alreadyOwnedErr *controllerutil.AlreadyOwnedError | ||
| assert.ErrorAs(t, err, &alreadyOwnedErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use require.ErrorAs instead of assert.ErrorAs after require.Error.
Per static analysis, error assertions should use require to fail fast and avoid nil pointer issues in subsequent assertions.
🔧 Suggested fix
err = m2.SetCurrent(obj)
require.Error(t, err)
var alreadyOwnedErr *controllerutil.AlreadyOwnedError
- assert.ErrorAs(t, err, &alreadyOwnedErr)
+ require.ErrorAs(t, err, &alreadyOwnedErr)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.Error(t, err) | |
| var alreadyOwnedErr *controllerutil.AlreadyOwnedError | |
| assert.ErrorAs(t, err, &alreadyOwnedErr) | |
| require.Error(t, err) | |
| var alreadyOwnedErr *controllerutil.AlreadyOwnedError | |
| require.ErrorAs(t, err, &alreadyOwnedErr) |
🧰 Tools
🪛 golangci-lint (2.5.0)
[error] 85-85: require-error: for error assertions use require
(testifylint)
🤖 Prompt for AI Agents
In `@ownerhandling/annotation_test.go` around lines 83 - 85, The test uses
require.Error(t, err) then calls assert.ErrorAs(t, err, &alreadyOwnedErr) which
can continue on failure; replace the assertion with require.ErrorAs(t, err,
&alreadyOwnedErr) so the test fails fast and avoids nil-pointer issues—update
the call to require.ErrorAs referencing the same variable alreadyOwnedErr and
error value err in the ownerhandling/annotation_test.go test.
| // GetCurrent returns a RevisionReference describing the current revision of obj. | ||
| // Returns nil if there is no current revision set. | ||
| func (m *annotationRevisionMetadata) GetCurrent(obj metav1.Object) bctypes.RevisionReference { | ||
| for _, ref := range m.getOwnerReferences(obj) { | ||
| if ref.Controller != nil && *ref.Controller { | ||
| // Convert to metav1.OwnerReference for consistent return type. | ||
| ownerRef := ref.ToMetaV1OwnerRef() | ||
|
|
||
| return &ownerRef | ||
| } | ||
| } | ||
|
|
||
| s.setOwnerReferences(obj, ownerRefs) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for GetCurrent usage across the codebase
echo "=== GetCurrent usage ==="
rg -n -B2 -A5 'GetCurrent\(' --type=go
echo -e "\n=== RevisionReference interface definition ==="
rg -n -B2 -A10 'type RevisionReference' --type=goRepository: package-operator/boxcutter
Length of output: 7421
🏁 Script executed:
#!/bin/bash
# Check if metav1.OwnerReference has a Namespace field
echo "=== Checking metav1.OwnerReference fields ==="
# First find where it's imported from
rg -n 'metav1\.OwnerReference' --type=go | head -20
echo -e "\n=== Check annotationOwnerRef structure ==="
rg -n -B2 -A15 'type annotationOwnerRef' --type=go
echo -e "\n=== Check ToMetaV1OwnerRef method ==="
rg -n -B2 -A15 'func.*ToMetaV1OwnerRef' --type=goRepository: package-operator/boxcutter
Length of output: 5624
🏁 Script executed:
#!/bin/bash
# Check how GetCurrent return value is used - what happens after the call
echo "=== GetCurrent usage in machinery/objects.go ==="
sed -n '475,495p' machinery/objects.go
echo -e "\n=== Search for namespace access after GetCurrent calls ==="
rg -n -B3 -A10 'GetCurrent\(' ownerhandling/annotation.go machinery/objects.go | head -60Repository: package-operator/boxcutter
Length of output: 2573
🏁 Script executed:
#!/bin/bash
# Check if metav1.OwnerReference can have/store namespace
echo "=== Search for Namespace in metav1.OwnerReference usage ==="
rg -n 'OwnerReference.*Namespace' --type=go
echo -e "\n=== Check if annotationOwnerRef namespace is used elsewhere ==="
rg -n '\.Namespace' ownerhandling/annotation.go | head -20Repository: package-operator/boxcutter
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Check the test for ToMetaV1OwnerRef to see if namespace is tested
echo "=== Test case expectations for ToMetaV1OwnerRef conversion ==="
sed -n '429,475p' ownerhandling/annotation_test.goRepository: package-operator/boxcutter
Length of output: 1361
Namespace information is lost in GetCurrent() return value.
ToMetaV1OwnerRef() deliberately omits the Namespace field from annotationOwnerRef when converting to metav1.OwnerReference, despite the comment at line 475-476 in machinery/objects.go stating the annotation approach is meant to "allow cross-namespace and cross-cluster refs." Since metav1.OwnerReference (a standard Kubernetes type for same-namespace references) doesn't have a namespace field, consumers of GetCurrent() cannot access namespace information, which contradicts the cross-namespace support goal of annotation-based ownership.
🤖 Prompt for AI Agents
In `@ownerhandling/annotation.go` around lines 166 - 179, GetCurrent currently
converts an annotationOwnerRef to metav1.OwnerReference via ToMetaV1OwnerRef
which drops Namespace; instead preserve namespace by returning a
RevisionReference that carries namespace information — either return the
original annotationOwnerRef (from getOwnerReferences) or construct/return a
wrapper implementing bctypes.RevisionReference that embeds the
metav1.OwnerReference plus a Namespace field. Update GetCurrent in
annotationRevisionMetadata and, if needed, adjust ToMetaV1OwnerRef or add a new
conversion method so the returned object from GetCurrent still exposes Namespace
(refer to GetCurrent, getOwnerReferences, annotationOwnerRef and
ToMetaV1OwnerRef).
| // NativeRevisionMetadata uses .metadata.ownerReferences for ownership tracking. | ||
| type NativeRevisionMetadata struct { | ||
| owner client.Object | ||
| scheme *runtime.Scheme | ||
| } | ||
|
|
||
| // NewNative returns a new OwnerStrategyNative instance. | ||
| func NewNative(scheme *runtime.Scheme) *OwnerStrategyNative { | ||
| return &OwnerStrategyNative{ | ||
| scheme: scheme, | ||
| // NewNativeRevisionMetadata creates a RevisionMetadata using native ownerReferences. | ||
| // If allowCrossNamespace is false, only objects in owner.GetNamespace() are allowed. | ||
| // Panics if owner has an empty UID (not persisted to cluster). | ||
| func NewNativeRevisionMetadata( | ||
| owner client.Object, | ||
| scheme *runtime.Scheme, | ||
| allowCrossNamespace bool, | ||
| ) *NativeRevisionMetadata { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Persist and honor allowCrossNamespace (currently ignored).
allowCrossNamespace (Line 33) is never stored or checked, and IsNamespaceAllowed (Line 92) always enforces same-namespace for namespaced owners. This breaks cross-namespace mode and matches the unused-parameter lint warning.
🛠️ Proposed fix
type NativeRevisionMetadata struct {
owner client.Object
scheme *runtime.Scheme
+ allowCrossNamespace bool
}
@@
return &NativeRevisionMetadata{
owner: owner,
scheme: scheme,
+ allowCrossNamespace: allowCrossNamespace,
}
}
@@
// If owner is cluster-scoped, all namespaces are allowed.
if len(ownerNs) == 0 {
return true
}
+ if m.allowCrossNamespace {
+ return true
+ }
// For namespaced owners, object must be in the same namespace.
return obj.GetNamespace() == ownerNs
}Also applies to: 91-101
🧰 Tools
🪛 golangci-lint (2.5.0)
[warning] 35-35: unused-parameter: parameter 'allowCrossNamespace' seems to be unused, consider removing or renaming it as _
(revive)
🤖 Prompt for AI Agents
In `@ownerhandling/native.go` around lines 23 - 36, The constructor
NewNativeRevisionMetadata currently ignores the allowCrossNamespace parameter
and IsNamespaceAllowed always enforces same-namespace; store allowCrossNamespace
on the NativeRevisionMetadata struct (add a field like allowCrossNamespace
bool), set it in NewNativeRevisionMetadata, and update the IsNamespaceAllowed
method to return true when allowCrossNamespace is true (or when the owner is
cluster-scoped) otherwise enforce that target namespace equals
owner.GetNamespace(); reference NativeRevisionMetadata,
NewNativeRevisionMetadata, and IsNamespaceAllowed when making these changes.
| // CopyReferences copies all revision metadata from objA to objB except the current revision marker. | ||
| // This is used when taking over control from a previous owner while preserving their watch references. | ||
| func (m *NativeRevisionMetadata) CopyReferences(objA, objB metav1.Object) { | ||
| // Copy owner references from A to B. | ||
| objB.SetOwnerReferences(objA.GetOwnerReferences()) | ||
| // Release controller on B (set all Controller fields to false). | ||
| ownerRefs := objB.GetOwnerReferences() | ||
| for i := range ownerRefs { | ||
| ownerRefs[i].Controller = ptr.To(false) | ||
| } | ||
| objB.SetOwnerReferences(ownerRefs) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid mutating objA when clearing controller flags.
objB.SetOwnerReferences(objA.GetOwnerReferences()) reuses the same backing slice. Mutating ownerRefs[i].Controller can inadvertently clear controller flags on objA. Clone before mutation.
🛠️ Proposed fix
- // Copy owner references from A to B.
- objB.SetOwnerReferences(objA.GetOwnerReferences())
- // Release controller on B (set all Controller fields to false).
- ownerRefs := objB.GetOwnerReferences()
+ // Copy owner references from A to B (clone to avoid aliasing).
+ ownerRefs := slices.Clone(objA.GetOwnerReferences())
+ // Release controller on B (set all Controller fields to false).
for i := range ownerRefs {
ownerRefs[i].Controller = ptr.To(false)
}
objB.SetOwnerReferences(ownerRefs)🤖 Prompt for AI Agents
In `@ownerhandling/native.go` around lines 103 - 114, The CopyReferences
implementation on NativeRevisionMetadata is mutating the same OwnerReferences
slice returned from objA by calling
objB.SetOwnerReferences(objA.GetOwnerReferences()) then changing
ownerRefs[i].Controller, which can mutate objA; instead, create a deep copy of
the owner references before mutating: obtain refs := objA.GetOwnerReferences(),
allocate a new slice of the same length, copy each metav1.OwnerReference into
the new slice (or append a struct literal per element) and set Controller =
ptr.To(false) on the copied elements, then call objB.SetOwnerReferences with
that new slice so objA is never modified.
| revOwnerMetadata := ownerhandling.NewNativeRevisionMetadata(revOwner, Scheme, false) | ||
| rev := boxcutter.Revision{ | ||
| Name: "rev-1", | ||
| Owner: revOwner, | ||
| Metadata: revOwnerMetadata, | ||
| Revision: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create metadata only after the owner UID exists.
Line 55 builds NativeRevisionMetadata before the owner is created. NewNativeRevisionMetadata panics if the UID is empty, so this will fail in integration runs. Move metadata (and revision construction) after Client.Create, or re-fetch to ensure UID is set.
✅ Suggested fix (move metadata creation after owner creation)
- revOwnerMetadata := ownerhandling.NewNativeRevisionMetadata(revOwner, Scheme, false)
- rev := boxcutter.Revision{
- Name: "rev-1",
- Metadata: revOwnerMetadata,
- Revision: 1,
- Phases: []boxcutter.Phase{
- {
- Name: "phase-1",
- Objects: []unstructured.Unstructured{*obj1},
- },
- {
- Name: "phase-2",
- Objects: []unstructured.Unstructured{*obj2},
- },
- },
- }
-
- // Owner has to be there first:
- err := Client.Create(ctx, revOwner)
+ // Owner has to be there first:
+ err := Client.Create(ctx, revOwner)
require.NoError(t, err)
+ require.NotEmpty(t, revOwner.GetUID())
+
+ revOwnerMetadata := ownerhandling.NewNativeRevisionMetadata(revOwner, Scheme, false)
+ rev := boxcutter.Revision{
+ Name: "rev-1",
+ Metadata: revOwnerMetadata,
+ Revision: 1,
+ Phases: []boxcutter.Phase{
+ {
+ Name: "phase-1",
+ Objects: []unstructured.Unstructured{*obj1},
+ },
+ {
+ Name: "phase-2",
+ Objects: []unstructured.Unstructured{*obj2},
+ },
+ },
+ }🤖 Prompt for AI Agents
In `@test/revision_engine_basic_test.go` around lines 55 - 59,
NewNativeRevisionMetadata is being called before the owner UID exists which
causes a panic when UID is empty; move creation of revOwnerMetadata and the
boxcutter.Revision rev until after you create the owner (i.e., after calling
Client.Create on revOwner) or re-fetch the owner to obtain its UID, then
construct revOwnerMetadata with ownerhandling.NewNativeRevisionMetadata and
build rev using that metadata so UID is non-empty.
Summary
The primary purpose of this refactor is to remove the hard-coded
association between a Revision and a client.Object. It allows the
creation of a RevisionMetadata implementation which does not require a
separate kubernetes object to exist for each Revision.
Change Type
Breaking Change
Check List Before Merging
Additional Information