diff --git a/apis/common/v1/resource.go b/apis/common/v1/resource.go index 02aef8f..4def267 100644 --- a/apis/common/v1/resource.go +++ b/apis/common/v1/resource.go @@ -120,22 +120,18 @@ const ( // IsResolutionPolicyOptional checks whether the resolution policy of relevant reference is Optional. func (p *Policy) IsResolutionPolicyOptional() bool { - if p != nil { - if p.Resolution != nil { - return *p.Resolution == ResolutionPolicyOptional - } + if p == nil || p.Resolution == nil { + return false } - return false + return *p.Resolution == ResolutionPolicyOptional } // IsResolvePolicyAlways checks whether the resolution policy of relevant reference is Always. func (p *Policy) IsResolvePolicyAlways() bool { - if p != nil { - if p.Resolve != nil { - return *p.Resolve == ResolvePolicyAlways - } + if p == nil || p.Resolve == nil { + return false } - return false + return *p.Resolve == ResolvePolicyAlways } // A Reference to a named object. @@ -174,6 +170,10 @@ type Selector struct { // MatchControllerRef ensures an object with the same controller reference // as the selecting object is selected. MatchControllerRef *bool `json:"matchControllerRef,omitempty"` + + // Policies of the referenced object. + // +optional + Policy *Policy `json:"policy,omitempty"` } // SetGroupVersionKind sets the Kind and APIVersion of a TypedReference. diff --git a/apis/common/v1/zz_generated.deepcopy.go b/apis/common/v1/zz_generated.deepcopy.go index a131319..5f8a7e3 100644 --- a/apis/common/v1/zz_generated.deepcopy.go +++ b/apis/common/v1/zz_generated.deepcopy.go @@ -459,6 +459,11 @@ func (in *Selector) DeepCopyInto(out *Selector) { *out = new(bool) **out = **in } + if in.Policy != nil { + in, out := &in.Policy, &out.Policy + *out = new(Policy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Selector. diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index 21bfce5..8c27378 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -114,11 +114,16 @@ type ResolutionRequest struct { // processed. func (rr ResolutionRequest) IsNoOp() bool { isAlways := false - if rr.Reference != nil { + if rr.Selector != nil { + if rr.Selector.Policy.IsResolvePolicyAlways() { + isAlways = true + } + } else if rr.Reference != nil { if rr.Reference.Policy.IsResolvePolicyAlways() { 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 @@ -162,12 +167,19 @@ type MultiResolutionRequest struct { // not be processed. func (rr MultiResolutionRequest) IsNoOp() bool { isAlways := false - for _, r := range rr.References { - if r.Policy.IsResolvePolicyAlways() { + if rr.Selector != nil { + if rr.Selector.Policy.IsResolvePolicyAlways() { isAlways = true - break + } + } else { + for _, r := range rr.References { + if r.Policy.IsResolvePolicyAlways() { + 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 @@ -227,32 +239,33 @@ func (r *APIResolver) Resolve(ctx context.Context, req ResolutionRequest) (Resol return ResolutionResponse{ResolvedValue: req.CurrentValue, ResolvedReference: req.Reference}, nil } - // The reference is already set - resolve it. - if req.Reference != nil { - if err := r.client.Get(ctx, types.NamespacedName{Name: req.Reference.Name}, req.To.Managed); err != nil { - return ResolutionResponse{}, getResolutionError(req.Reference.Policy, errors.Wrap(err, errGetManaged)) + // The selector is set - resolve it. + if req.Selector != nil { + if err := r.client.List(ctx, req.To.List, client.MatchingLabels(req.Selector.MatchLabels)); err != nil { + return ResolutionResponse{}, getResolutionError(req.Selector.Policy, errors.Wrap(err, errListManaged)) } - rsp := ResolutionResponse{ResolvedValue: req.Extract(req.To.Managed), ResolvedReference: req.Reference} - return rsp, getResolutionError(req.Reference.Policy, rsp.Validate()) - } + for _, to := range req.To.List.GetItems() { + if ControllersMustMatch(req.Selector) && !meta.HaveSameController(r.from, to) { + continue + } - // The reference was not set, but a selector was. Select a reference. - if err := r.client.List(ctx, req.To.List, client.MatchingLabels(req.Selector.MatchLabels)); err != nil { - return ResolutionResponse{}, errors.Wrap(err, errListManaged) - } - - for _, to := range req.To.List.GetItems() { - if ControllersMustMatch(req.Selector) && !meta.HaveSameController(r.from, to) { - continue + rsp := ResolutionResponse{ResolvedValue: req.Extract(to), ResolvedReference: &xpv1.Reference{Name: to.GetName()}} + return rsp, getResolutionError(req.Selector.Policy, rsp.Validate()) } - rsp := ResolutionResponse{ResolvedValue: req.Extract(to), ResolvedReference: &xpv1.Reference{Name: to.GetName()}} - return rsp, rsp.Validate() + // We couldn't resolve anything. + return ResolutionResponse{}, getResolutionError(req.Selector.Policy, errors.New(errNoMatches)) } - // We couldn't resolve anything. - return ResolutionResponse{}, errors.New(errNoMatches) + // The selector was not set, check the reference. + if err := r.client.Get(ctx, types.NamespacedName{Name: req.Reference.Name}, req.To.Managed); err != nil { + return ResolutionResponse{}, getResolutionError(req.Reference.Policy, errors.Wrap(err, errGetManaged)) + } + + rsp := ResolutionResponse{ResolvedValue: req.Extract(req.To.Managed), ResolvedReference: req.Reference} + return rsp, getResolutionError(req.Reference.Policy, rsp.Validate()) + } // ResolveMultiple resolves the supplied MultiResolutionRequest. The returned @@ -264,39 +277,40 @@ func (r *APIResolver) ResolveMultiple(ctx context.Context, req MultiResolutionRe return MultiResolutionResponse{ResolvedValues: req.CurrentValues, ResolvedReferences: req.References}, nil } - // The references are already set - resolve them. - if len(req.References) > 0 { - vals := make([]string, len(req.References)) - for i := range req.References { - if err := r.client.Get(ctx, types.NamespacedName{Name: req.References[i].Name}, req.To.Managed); err != nil { - return MultiResolutionResponse{}, getResolutionError(req.References[i].Policy, errors.Wrap(err, errGetManaged)) + // The selector is set - resolve it. + if req.Selector != nil { + if err := r.client.List(ctx, req.To.List, client.MatchingLabels(req.Selector.MatchLabels)); err != nil { + return MultiResolutionResponse{}, getResolutionError(req.Selector.Policy, errors.Wrap(err, errListManaged)) + } + + items := req.To.List.GetItems() + refs := make([]xpv1.Reference, 0, len(items)) + vals := make([]string, 0, len(items)) + for _, to := range req.To.List.GetItems() { + if ControllersMustMatch(req.Selector) && !meta.HaveSameController(r.from, to) { + continue } - vals[i] = req.Extract(req.To.Managed) + + vals = append(vals, req.Extract(to)) + refs = append(refs, xpv1.Reference{Name: to.GetName()}) } - rsp := MultiResolutionResponse{ResolvedValues: vals, ResolvedReferences: req.References} - return rsp, rsp.Validate() + rsp := MultiResolutionResponse{ResolvedValues: vals, ResolvedReferences: refs} + return rsp, getResolutionError(req.Selector.Policy, rsp.Validate()) } - // No references were set, but a selector was. Select and resolve references. - if err := r.client.List(ctx, req.To.List, client.MatchingLabels(req.Selector.MatchLabels)); err != nil { - return MultiResolutionResponse{}, errors.Wrap(err, errListManaged) - } - - items := req.To.List.GetItems() - refs := make([]xpv1.Reference, 0, len(items)) - vals := make([]string, 0, len(items)) - for _, to := range req.To.List.GetItems() { - if ControllersMustMatch(req.Selector) && !meta.HaveSameController(r.from, to) { - continue + // The selector was not set, check the references. + vals := make([]string, len(req.References)) + for i := range req.References { + if err := r.client.Get(ctx, types.NamespacedName{Name: req.References[i].Name}, req.To.Managed); err != nil { + return MultiResolutionResponse{}, getResolutionError(req.References[i].Policy, errors.Wrap(err, errGetManaged)) } - - vals = append(vals, req.Extract(to)) - refs = append(refs, xpv1.Reference{Name: to.GetName()}) + vals[i] = req.Extract(req.To.Managed) } - rsp := MultiResolutionResponse{ResolvedValues: vals, ResolvedReferences: refs} + rsp := MultiResolutionResponse{ResolvedValues: vals, ResolvedReferences: req.References} return rsp, rsp.Validate() + } func getResolutionError(p *xpv1.Policy, err error) error { diff --git a/pkg/reference/reference_test.go b/pkg/reference/reference_test.go index d8f788b..1082cea 100644 --- a/pkg/reference/reference_test.go +++ b/pkg/reference/reference_test.go @@ -138,8 +138,8 @@ 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" + + "AlwaysResolveReference": { + reason: "Should not return early if the current value is non-zero, when the resolve policy is set to" + "Always", c: &test.MockClient{ MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { @@ -234,7 +234,7 @@ func TestResolve(t *testing.T) { }, }, }, - "OptionalPolicy": { + "OptionalReference": { reason: "No error should be returned when the resolution policy is Optional", c: &test.MockClient{ MockGet: test.NewMockGetFn(nil), @@ -287,6 +287,25 @@ func TestResolve(t *testing.T) { err: errors.New(errNoMatches), }, }, + "OptionalSelector": { + reason: "No error should be returned when the resolution policy is Optional", + c: &test.MockClient{ + MockList: test.NewMockListFn(nil), + }, + from: &fake.Managed{}, + args: args{ + req: ResolutionRequest{ + Selector: &xpv1.Selector{ + Policy: &xpv1.Policy{Resolution: &optionalPolicy}, + }, + To: To{List: &FakeManagedList{}}, + }, + }, + want: want{ + rsp: ResolutionResponse{}, + err: nil, + }, + }, "SuccessfulSelect": { reason: "A managed resource with a matching controller reference should be selected and returned", c: &test.MockClient{ @@ -313,6 +332,35 @@ func TestResolve(t *testing.T) { err: nil, }, }, + "AlwaysResolveSelector": { + reason: "Should not return early if the current value is non-zero, when the resolve policy is set to" + + "Always", + c: &test.MockClient{ + MockList: test.NewMockListFn(nil), + }, + from: controlled, + args: args{ + req: ResolutionRequest{ + Selector: &xpv1.Selector{ + MatchControllerRef: func() *bool { t := true; return &t }(), + Policy: &xpv1.Policy{Resolve: &alwaysPolicy}, + }, + To: To{List: &FakeManagedList{Items: []resource.Managed{ + &fake.Managed{}, // A resource that does not match. + controlled, // A resource with a matching controller reference. + }}}, + Extract: ExternalName(), + CurrentValue: "oldValue", + }, + }, + want: want{ + rsp: ResolutionResponse{ + ResolvedValue: value, + ResolvedReference: &xpv1.Reference{Name: value}, + }, + err: nil, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -379,8 +427,8 @@ 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" + + "AlwaysResolveReference": { + reason: "Should not return early if the current value is non-zero, when the resolve policy is set to" + "Always", c: &test.MockClient{ MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { @@ -476,7 +524,7 @@ func TestResolveMultiple(t *testing.T) { }, }, }, - "OptionalPolicy": { + "OptionalReference": { reason: "No error should be returned when the resolution policy is Optional", c: &test.MockClient{ MockGet: test.NewMockGetFn(nil), @@ -530,6 +578,25 @@ func TestResolveMultiple(t *testing.T) { err: errors.New(errNoMatches), }, }, + "OptionalSelector": { + reason: "No error should be returned when the resolution policy is Optional", + c: &test.MockClient{ + MockList: test.NewMockListFn(nil), + }, + from: &fake.Managed{}, + args: args{ + req: MultiResolutionRequest{ + Selector: &xpv1.Selector{ + Policy: &xpv1.Policy{Resolution: &optionalPolicy}, + }, + To: To{List: &FakeManagedList{}}, + }, + }, + want: want{ + rsp: MultiResolutionResponse{}, + err: nil, + }, + }, "SuccessfulSelect": { reason: "A managed resource with a matching controller reference should be selected and returned", c: &test.MockClient{ @@ -556,6 +623,35 @@ func TestResolveMultiple(t *testing.T) { err: nil, }, }, + "AlwaysResolveSelector": { + reason: "Should not return early if the current value is non-zero, when the resolve policy is set to" + + "Always", + c: &test.MockClient{ + MockList: test.NewMockListFn(nil), + }, + from: controlled, + args: args{ + req: MultiResolutionRequest{ + Selector: &xpv1.Selector{ + MatchControllerRef: func() *bool { t := true; return &t }(), + Policy: &xpv1.Policy{Resolve: &alwaysPolicy}, + }, + To: To{List: &FakeManagedList{Items: []resource.Managed{ + &fake.Managed{}, // A resource that does not match. + controlled, // A resource with a matching controller reference. + }}}, + Extract: ExternalName(), + CurrentValues: []string{"oldValue"}, + }, + }, + want: want{ + rsp: MultiResolutionResponse{ + ResolvedValues: []string{value}, + ResolvedReferences: []xpv1.Reference{{Name: value}}, + }, + err: nil, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) {