diff --git a/apis/common/v1/resource.go b/apis/common/v1/resource.go index 4a89432..28c366a 100644 --- a/apis/common/v1/resource.go +++ b/apis/common/v1/resource.go @@ -85,19 +85,39 @@ const ( // When the ReferenceResolutionPolicy is set to ReferencePolicyOptional the // execution could continue even if the reference cannot be resolved. ReferencePolicyOptional ReferenceResolutionPolicy = "Optional" + + // ReferencePolicyAlways is a resolution option. + // When the ReferenceResolutionPolicy is set to ReferencePolicyAlways the + // reference will be tried to resolve for every reconcile loop. + ReferencePolicyAlways ReferenceResolutionPolicy = "Always" ) // A Reference to a named object. type Reference struct { // Name of the referenced object. Name string `json:"name"` - // Policy of the referenced object. - Policy ReferenceResolutionPolicy `json:"policy,omitempty"` + // Policies of the referenced object. + Policies []ReferenceResolutionPolicy `json:"policies,omitempty"` } // IsReferenceResolutionPolicyOptional checks whether the resolution policy of relevant reference is Optional. func (in *Reference) IsReferenceResolutionPolicyOptional() bool { - return in.Policy == ReferencePolicyOptional + for _, policy := range in.Policies { + if policy == ReferencePolicyOptional { + return true + } + } + return false +} + +// IsReferenceResolutionPolicyAlways checks whether the resolution policy of relevant reference is Always. +func (in *Reference) IsReferenceResolutionPolicyAlways() bool { + for _, policy := range in.Policies { + if policy == ReferencePolicyAlways { + return true + } + } + return false } // A TypedReference refers to an object by Name, Kind, and APIVersion. It is diff --git a/apis/common/v1/zz_generated.deepcopy.go b/apis/common/v1/zz_generated.deepcopy.go index d7cae60..8a322e7 100644 --- a/apis/common/v1/zz_generated.deepcopy.go +++ b/apis/common/v1/zz_generated.deepcopy.go @@ -248,7 +248,7 @@ func (in *ProviderConfigStatus) DeepCopy() *ProviderConfigStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProviderConfigUsage) DeepCopyInto(out *ProviderConfigUsage) { *out = *in - out.ProviderConfigReference = in.ProviderConfigReference + in.ProviderConfigReference.DeepCopyInto(&out.ProviderConfigReference) out.ResourceReference = in.ResourceReference } @@ -273,7 +273,7 @@ func (in *PublishConnectionDetailsTo) DeepCopyInto(out *PublishConnectionDetails if in.SecretStoreConfigRef != nil { in, out := &in.SecretStoreConfigRef, &out.SecretStoreConfigRef *out = new(Reference) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -290,6 +290,11 @@ func (in *PublishConnectionDetailsTo) DeepCopy() *PublishConnectionDetailsTo { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Reference) DeepCopyInto(out *Reference) { *out = *in + if in.Policies != nil { + in, out := &in.Policies, &out.Policies + *out = make([]ReferenceResolutionPolicy, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Reference. @@ -318,12 +323,12 @@ func (in *ResourceSpec) DeepCopyInto(out *ResourceSpec) { if in.ProviderConfigReference != nil { in, out := &in.ProviderConfigReference, &out.ProviderConfigReference *out = new(Reference) - **out = **in + (*in).DeepCopyInto(*out) } if in.ProviderReference != nil { in, out := &in.ProviderReference, &out.ProviderReference *out = new(Reference) - **out = **in + (*in).DeepCopyInto(*out) } } diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index ea032a1..24b2fb4 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -113,10 +113,17 @@ type ResolutionRequest struct { // IsNoOp returns true if the supplied ResolutionRequest cannot or should not be // processed. func (rr ResolutionRequest) IsNoOp() bool { - // We don't resolve values that are already set; we effectively cache - // resolved values. The CR author can invalidate the cache and trigger a new - // resolution by explicitly clearing the resolved value. - if rr.CurrentValue != "" { + isAlways := false + if rr.Reference != nil { + if rr.Reference.IsReferenceResolutionPolicyAlways() { + isAlways = true + } + } + // We don't resolve values that are already set (if reference resolution policy + // is not set to Always); we effectively cache resolved values. The CR author + // can invalidate the cache and trigger a new resolution by explicitly clearing + // the resolved value. + if rr.CurrentValue != "" && !isAlways { return true } @@ -154,12 +161,19 @@ type MultiResolutionRequest struct { // IsNoOp returns true if the supplied MultiResolutionRequest cannot or should // not be processed. func (rr MultiResolutionRequest) IsNoOp() bool { - // We don't resolve values that are already set; we effectively cache - // resolved values. The CR author can invalidate the cache and trigger a new - // resolution by explicitly clearing the resolved values. This is a little - // unintuitive for the APIMultiResolver but mimics the UX of the APIResolver - // and simplifies the overall mental model. - if len(rr.CurrentValues) > 0 { + isAlways := false + for _, r := range rr.References { + if r.IsReferenceResolutionPolicyAlways() { + isAlways = true + break + } + } + // We don't resolve values that are already set (if reference resolution policy + // is not set to Always); we effectively cache resolved values. The CR author + // can invalidate the cache and trigger a new resolution by explicitly clearing + // the resolved values. This is a little unintuitive for the APIMultiResolver + // but mimics the UX of the APIResolver and simplifies the overall mental model. + if len(rr.CurrentValues) > 0 && !isAlways { return true } diff --git a/pkg/reference/reference_test.go b/pkg/reference/reference_test.go index a105747..93a8603 100644 --- a/pkg/reference/reference_test.go +++ b/pkg/reference/reference_test.go @@ -91,7 +91,8 @@ func TestResolve(t *testing.T) { now := metav1.Now() value := "coolv" ref := &xpv1.Reference{Name: "cool"} - optionalRef := &xpv1.Reference{Name: "cool", Policy: xpv1.ReferencePolicyOptional} + optionalRef := &xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyOptional}} + alwaysRef := &xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyAlways}} controlled := &fake.Managed{} controlled.SetName(value) @@ -135,6 +136,32 @@ func TestResolve(t *testing.T) { err: nil, }, }, + "AlwaysResolve": { + reason: "Should not return early if the current value is non-zero, when the resolution policy is set to" + + "Always", + c: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + meta.SetExternalName(obj.(metav1.Object), value) + return nil + }), + }, + from: &fake.Managed{}, + args: args{ + req: ResolutionRequest{ + Reference: alwaysRef, + To: To{Managed: &fake.Managed{}}, + Extract: ExternalName(), + CurrentValue: "oldValue", + }, + }, + want: want{ + rsp: ResolutionResponse{ + ResolvedValue: value, + ResolvedReference: alwaysRef, + }, + err: nil, + }, + }, "Unresolvable": { reason: "Should return early if neither a reference or selector were provided", from: &fake.Managed{}, @@ -303,7 +330,8 @@ func TestResolveMultiple(t *testing.T) { now := metav1.Now() value := "coolv" ref := xpv1.Reference{Name: "cool"} - optionalRef := xpv1.Reference{Name: "cool", Policy: xpv1.ReferencePolicyOptional} + optionalRef := xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyOptional}} + alwaysRef := xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyAlways}} controlled := &fake.Managed{} controlled.SetName(value) @@ -347,6 +375,32 @@ func TestResolveMultiple(t *testing.T) { err: nil, }, }, + "AlwaysResolve": { + reason: "Should not return early if the current value is non-zero, when the resolution policy is set to" + + "Always", + c: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + meta.SetExternalName(obj.(metav1.Object), value) + return nil + }), + }, + from: &fake.Managed{}, + args: args{ + req: MultiResolutionRequest{ + References: []xpv1.Reference{alwaysRef}, + To: To{Managed: &fake.Managed{}}, + Extract: ExternalName(), + CurrentValues: []string{"oldValue"}, + }, + }, + want: want{ + rsp: MultiResolutionResponse{ + ResolvedValues: []string{value}, + ResolvedReferences: []xpv1.Reference{alwaysRef}, + }, + err: nil, + }, + }, "Unresolvable": { reason: "Should return early if neither a reference or selector were provided", from: &fake.Managed{}, diff --git a/pkg/resource/providerconfig.go b/pkg/resource/providerconfig.go index 5f15978..93ab494 100644 --- a/pkg/resource/providerconfig.go +++ b/pkg/resource/providerconfig.go @@ -20,6 +20,8 @@ import ( "context" "os" + "github.com/google/go-cmp/cmp" + "github.com/spf13/afero" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -153,7 +155,7 @@ func (u *ProviderConfigUsageTracker) Track(ctx context.Context, mg Managed) erro err := u.c.Apply(ctx, pcu, MustBeControllableBy(mg.GetUID()), AllowUpdateIf(func(current, _ runtime.Object) bool { - return current.(ProviderConfigUsage).GetProviderConfigReference() != pcu.GetProviderConfigReference() + return !cmp.Equal(current.(ProviderConfigUsage).GetProviderConfigReference(), pcu.GetProviderConfigReference()) }), ) return errors.Wrap(Ignore(IsNotAllowed, err), errApplyPCU)