Skip to content

Commit

Permalink
Merge pull request crossplane#5474 from turkenh/only-check-kind
Browse files Browse the repository at this point in the history
Allow group changes in composed templates
  • Loading branch information
turkenh authored Mar 14, 2024
2 parents b565aac + d347ff3 commit 47d8148
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ const (
errMarshalProtoStruct = "cannot marshal protobuf Struct to JSON"
errSetControllerRef = "cannot set controller reference"

errFmtKindOrGroupChanged = "cannot change the kind or group of a composed resource from %s to %s (possible composed resource template mismatch)"
errFmtNamePrefixLabel = "cannot find top-level composite resource name label %q in composite resource metadata"
errFmtKindChanged = "cannot change the kind of a composed resource from %s to %s (possible composed resource template mismatch)"
errFmtNamePrefixLabel = "cannot find top-level composite resource name label %q in composite resource metadata"

// TODO(negz): Include more detail such as field paths if they exist.
// Perhaps require each patch type to have a String() method to help
Expand Down Expand Up @@ -61,16 +61,18 @@ func RenderFromJSON(o resource.Object, data []byte) error {
o.SetName(name)
o.SetNamespace(namespace)

// This resource already had a GK (probably because it already exists), but
// This resource already had a Kind (probably because it already exists), but
// when we rendered its template it changed. This shouldn't happen. Either
// someone changed the kind or group in the template, or we're trying to use the
// someone changed the kind in the template, or we're trying to use the
// wrong template (e.g. because the order of an array of anonymous templates
// changed).
// Please note, we don't check for version changes, as versions can change. For example,
// if a composed resource was created with a template that has a version of "v1alpha1",
// and then the template is updated to "v1beta1", the composed resource will still be valid.
if !gvk.Empty() && o.GetObjectKind().GroupVersionKind().GroupKind() != gvk.GroupKind() {
return errors.Errorf(errFmtKindOrGroupChanged, gvk, o.GetObjectKind().GroupVersionKind())
// We also don't check for group changes, as groups can change during
// migrations.
if !gvk.Empty() && o.GetObjectKind().GroupVersionKind().Kind != gvk.Kind {
return errors.Errorf(errFmtKindChanged, gvk, o.GetObjectKind().GroupVersionKind())
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,38 +62,37 @@ func TestRenderFromJSON(t *testing.T) {
err: errors.Wrap(errInvalidChar, errUnmarshalJSON),
},
},
"ExistingGroupChanged": {
reason: "We should return an error if unmarshalling the base template changed the composed resource's group.",
"ExistingKindChanged": {
reason: "We should return an error if unmarshalling the base template changed the composed resource's kind.",
args: args{
o: composed.New(composed.FromReference(corev1.ObjectReference{
APIVersion: "example.org/v1",
Kind: "Potato",
})),
data: []byte(`{"apiVersion": "foo.io/v1", "kind": "Potato"}`),
data: []byte(`{"apiVersion": "example.org/v1", "kind": "Different"}`),
},
want: want{
o: composed.New(composed.FromReference(corev1.ObjectReference{
APIVersion: "foo.io/v1",
Kind: "Potato",
APIVersion: "example.org/v1",
Kind: "Different",
})),
err: errors.Errorf(errFmtKindOrGroupChanged, "example.org/v1, Kind=Potato", "foo.io/v1, Kind=Potato"),
err: errors.Errorf(errFmtKindChanged, "example.org/v1, Kind=Potato", "example.org/v1, Kind=Different"),
},
},
"ExistingKindChanged": {
reason: "We should return an error if unmarshalling the base template changed the composed resource's kind.",
"GroupCanChange": {
reason: "We should accept group changes in the base template.",
args: args{
o: composed.New(composed.FromReference(corev1.ObjectReference{
APIVersion: "example.org/v1",
Kind: "Potato",
})),
data: []byte(`{"apiVersion": "example.org/v1", "kind": "Different"}`),
data: []byte(`{"apiVersion": "foo.io/v1", "kind": "Potato"}`),
},
want: want{
o: composed.New(composed.FromReference(corev1.ObjectReference{
APIVersion: "example.org/v1",
Kind: "Different",
APIVersion: "foo.io/v1",
Kind: "Potato",
})),
err: errors.Errorf(errFmtKindOrGroupChanged, "example.org/v1, Kind=Potato", "example.org/v1, Kind=Different"),
},
},
"VersionCanChange": {
Expand Down

0 comments on commit 47d8148

Please sign in to comment.