diff --git a/pkg/controller/apiextensions/composite/api.go b/pkg/controller/apiextensions/composite/api.go index 69ca605dc47..8b376e95182 100644 --- a/pkg/controller/apiextensions/composite/api.go +++ b/pkg/controller/apiextensions/composite/api.go @@ -30,6 +30,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" + "github.com/crossplane/crossplane/pkg/controller/apiextensions/composite/composed" ) // Error strings. @@ -217,6 +218,26 @@ func (s *EnforcedCompositionSelector) SelectComposition(_ context.Context, cp re return nil } +// NewConfiguratorChain returns a new *ConfiguratorChain. +func NewConfiguratorChain(l ...Configurator) *ConfiguratorChain { + return &ConfiguratorChain{list: l} +} + +// ConfiguratorChain executes the Configurators in given order. +type ConfiguratorChain struct { + list []Configurator +} + +// Configure calls Configure function of every Configurator in the list. +func (cc *ConfiguratorChain) Configure(ctx context.Context, cp resource.Composite, comp *v1alpha1.Composition) error { + for _, c := range cc.list { + if err := c.Configure(ctx, cp, comp); err != nil { + return err + } + } + return nil +} + // NewAPIConfigurator returns a Configurator that configures a // composite resource using its composition. func NewAPIConfigurator(c client.Client) *APIConfigurator { @@ -252,3 +273,24 @@ func (c *APIConfigurator) Configure(ctx context.Context, cp resource.Composite, return errors.Wrap(c.client.Update(ctx, cp), errUpdateComposite) } + +// NewAPINamingConfigurator returns a Configurator that sets the root name prefixKu +// to its own name if it is not already set. +func NewAPINamingConfigurator(c client.Client) *APINamingConfigurator { + return &APINamingConfigurator{client: c} +} + +// An APINamingConfigurator sets the root name prefix to its own name if it is not +// already set. +type APINamingConfigurator struct { + client client.Client +} + +// Configure the supplied composite resource's root name prefix. +func (c *APINamingConfigurator) Configure(ctx context.Context, cp resource.Composite, _ *v1alpha1.Composition) error { + if cp.GetLabels()[composed.LabelKeyNamePrefixForComposed] != "" { + return nil + } + meta.AddLabels(cp, map[string]string{composed.LabelKeyNamePrefixForComposed: cp.GetName()}) + return errors.Wrap(c.client.Update(ctx, cp), errUpdateComposite) +} diff --git a/pkg/controller/apiextensions/composite/api_test.go b/pkg/controller/apiextensions/composite/api_test.go index 99e274ee84a..4f3b54423e8 100644 --- a/pkg/controller/apiextensions/composite/api_test.go +++ b/pkg/controller/apiextensions/composite/api_test.go @@ -36,6 +36,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" + "github.com/crossplane/crossplane/pkg/controller/apiextensions/composite/composed" ) var errBoom = errors.New("boom") @@ -541,3 +542,54 @@ func TestAPIEnforcedCompositionSelector(t *testing.T) { }) } } + +func TestAPINamingConfigurator(t *testing.T) { + type args struct { + kube client.Client + cp resource.Composite + } + type want struct { + cp resource.Composite + err error + } + + cases := map[string]struct { + reason string + args + want + }{ + "LabelAlreadyExists": { + reason: "No operation should be done if the name prefix is already given", + args: args{ + cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{composed.LabelKeyNamePrefixForComposed: "given"}}}, + }, + want: want{ + cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{composed.LabelKeyNamePrefixForComposed: "given"}}}, + }, + }, + "AssignedName": { + reason: "Its own name should be used as name prefix if it is not given", + args: args{ + kube: &test.MockClient{ + MockUpdate: test.NewMockUpdateFn(nil), + }, + cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp"}}, + }, + want: want{ + cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp", Labels: map[string]string{composed.LabelKeyNamePrefixForComposed: "cp"}}}, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := NewAPINamingConfigurator(tc.args.kube) + err := c.Configure(context.Background(), tc.args.cp, nil) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConfigure(...): -want, +got:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.cp, tc.args.cp); diff != "" { + t.Errorf("\n%s\nConfigure(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/pkg/controller/apiextensions/composite/composed/api.go b/pkg/controller/apiextensions/composite/composed/api.go index 62d82576efd..74d335361ba 100644 --- a/pkg/controller/apiextensions/composite/composed/api.go +++ b/pkg/controller/apiextensions/composite/composed/api.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/util/json" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" @@ -32,9 +33,17 @@ import ( ) const ( - errUnmarshal = "cannot unmarshal base template" - errFmtPatch = "cannot apply the patch at index %d" - errGetSecret = "cannot get connection secret of composed resource" + errUnmarshal = "cannot unmarshal base template" + errFmtPatch = "cannot apply the patch at index %d" + errGetSecret = "cannot get connection secret of composed resource" + errNamePrefix = "name prefix is not found in labels" +) + +// Label keys. +const ( + LabelKeyNamePrefixForComposed = "crossplane.io/composite" + LabelKeyRequirementName = "crossplane.io/requirement-name" + LabelKeyRequirementNamespace = "crossplane.io/requirement-namespace" ) // ConfigureFn is a function that implements Configurator interface. @@ -54,14 +63,25 @@ func (*DefaultConfigurator) Configure(cp resource.Composite, cd resource.Compose // Any existing name will be overwritten when we unmarshal the template. We // store it here so that we can reset it after unmarshalling. name := cd.GetName() + namespace := cd.GetNamespace() if err := json.Unmarshal(t.Base.Raw, cd); err != nil { return errors.Wrap(err, errUnmarshal) } + if cp.GetLabels()[LabelKeyNamePrefixForComposed] == "" { + return errors.New(errNamePrefix) + } + // This label will be used if composed resource is yet another composite. + meta.AddLabels(cd, map[string]string{ + LabelKeyNamePrefixForComposed: cp.GetLabels()[LabelKeyNamePrefixForComposed], + LabelKeyRequirementName: cp.GetLabels()[LabelKeyRequirementName], + LabelKeyRequirementNamespace: cp.GetLabels()[LabelKeyRequirementNamespace], + }) // Unmarshalling the template will overwrite any existing fields, so we must // restore the existing name, if any. We also set generate name in case we // haven't yet named this composed resource. - cd.SetGenerateName(cp.GetName() + "-") + cd.SetGenerateName(cp.GetLabels()[LabelKeyNamePrefixForComposed] + "-") cd.SetName(name) + cd.SetNamespace(namespace) return nil } diff --git a/pkg/controller/apiextensions/composite/composed/api_test.go b/pkg/controller/apiextensions/composite/composed/api_test.go index c23180a702e..ecc443b10b9 100644 --- a/pkg/controller/apiextensions/composite/composed/api_test.go +++ b/pkg/controller/apiextensions/composite/composed/api_test.go @@ -71,15 +71,35 @@ func TestConfigure(t *testing.T) { err: errors.Wrap(errors.New("invalid character 'o' looking for beginning of value"), errUnmarshal), }, }, + "NoLabel": { + reason: "The name prefix label has to be set", + args: args{ + cp: &fake.Composite{}, + cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd"}}, + t: v1alpha1.ComposedTemplate{Base: runtime.RawExtension{Raw: tmpl}}, + }, + want: want{ + cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd"}}, + err: errors.New(errNamePrefix), + }, + }, "Success": { reason: "Configuration should result in the right object with correct generateName", args: args{ - cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp"}}, + cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + LabelKeyNamePrefixForComposed: "ola", + LabelKeyRequirementName: "rola", + LabelKeyRequirementNamespace: "rolans", + }}}, cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd"}}, t: v1alpha1.ComposedTemplate{Base: runtime.RawExtension{Raw: tmpl}}, }, want: want{ - cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd", GenerateName: "cp-"}}, + cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd", GenerateName: "ola-", Labels: map[string]string{ + LabelKeyNamePrefixForComposed: "ola", + LabelKeyRequirementName: "rola", + LabelKeyRequirementNamespace: "rolans", + }}}, }, }, } diff --git a/pkg/controller/apiextensions/composite/reconciler.go b/pkg/controller/apiextensions/composite/reconciler.go index aef481a7bb4..28b88c8a49e 100644 --- a/pkg/controller/apiextensions/composite/reconciler.go +++ b/pkg/controller/apiextensions/composite/reconciler.go @@ -177,7 +177,7 @@ func NewReconciler(mgr manager.Manager, of resource.CompositeKind, opts ...Recon composite: compositeResource{ CompositionSelector: NewAPILabelSelectorResolver(kube), - Configurator: NewAPIConfigurator(kube), + Configurator: NewConfiguratorChain(NewAPINamingConfigurator(kube), NewAPIConfigurator(kube)), ConnectionPublisher: NewAPIFilteredSecretPublisher(kube, []string{}), }, diff --git a/pkg/controller/apiextensions/requirement/configurator.go b/pkg/controller/apiextensions/requirement/configurator.go index 6374050b15e..c2f0fc834dc 100644 --- a/pkg/controller/apiextensions/requirement/configurator.go +++ b/pkg/controller/apiextensions/requirement/configurator.go @@ -26,17 +26,23 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite" "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/requirement" + + "github.com/crossplane/crossplane/pkg/controller/apiextensions/composite/composed" ) // Configure the supplied composite resource. The composite resource name is -// derived from the supplied requirement, as {namespace}-{name}-{random-string}. +// derived from the supplied requirement, as {name}-{random-string}. // The requirement's external name annotation, if any, is propagated to the // composite resource. func Configure(_ context.Context, rq resource.Requirement, cp resource.Composite) error { - cp.SetGenerateName(fmt.Sprintf("%s-%s-", rq.GetNamespace(), rq.GetName())) + cp.SetGenerateName(fmt.Sprintf("%s-", rq.GetName())) if meta.GetExternalName(rq) != "" { meta.SetExternalName(cp, meta.GetExternalName(rq)) } + meta.AddLabels(cp, map[string]string{ + composed.LabelKeyRequirementName: rq.GetName(), + composed.LabelKeyRequirementNamespace: rq.GetNamespace(), + }) urq, ok := rq.(*requirement.Unstructured) if !ok {