From b9c55196e65275598ff1a18973949fae1bc4f9a6 Mon Sep 17 00:00:00 2001 From: Dambach Date: Tue, 29 Sep 2026 11:54:44 +0200 Subject: [PATCH 1/3] Observe tags --- .../serviceinstance/service_instance.yaml | 5 ++ .../serviceinstance/serviceinstance.go | 42 ++++++++++ .../serviceinstance/serviceinstance_test.go | 78 +++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/examples/serviceinstance/service_instance.yaml b/examples/serviceinstance/service_instance.yaml index 8ecba463..718d3fce 100644 --- a/examples/serviceinstance/service_instance.yaml +++ b/examples/serviceinstance/service_instance.yaml @@ -15,3 +15,8 @@ spec: servicePlan: offering: destination plan: lite + # tags are applied to the instance and surfaced in apps' VCAP_SERVICES. + # Omitting this field or setting it to [] results in no tags. + tags: + - prod + - eu-central diff --git a/internal/clients/serviceinstance/serviceinstance.go b/internal/clients/serviceinstance/serviceinstance.go index b11e1427..0190ec41 100644 --- a/internal/clients/serviceinstance/serviceinstance.go +++ b/internal/clients/serviceinstance/serviceinstance.go @@ -10,6 +10,7 @@ import ( "github.com/cloudfoundry/go-cfclient/v3/resource" xpresource "github.com/crossplane/crossplane-runtime/v2/pkg/resource" "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" @@ -178,6 +179,7 @@ func (c *Client) createManaged(ctx context.Context, mg xpresource.Managed, spec opt := resource.NewServiceInstanceCreateManaged(*spec.Name, *spec.Space, *spec.ServicePlan.ID) opt.Metadata = metadata.BuildMetadata(mg, spec.Labels, spec.Annotations) + opt.WithTags(toTagSlice(spec.Tags)) if params != nil { opt.Parameters = ¶ms @@ -204,6 +206,8 @@ func (c *Client) createUserProvided(ctx context.Context, mg xpresource.Managed, // create the service instance opt := resource.NewServiceInstanceCreateUserProvided(*spec.Name, *spec.Space) opt.Metadata = metadata.BuildMetadata(mg, spec.Labels, spec.Annotations) + opt.WithTags(toTagSlice(spec.Tags)) + si, err := c.CreateUserProvided(ctx, opt) if err != nil { return nil, err @@ -252,6 +256,8 @@ func (c *Client) updateManaged(ctx context.Context, observed *resource.ServiceIn upd.WithParameters(params) } + upd.WithTags(toTagSlice(desired.Tags)) + upd.Metadata = metadata.BuildMetadata(mg, desired.Labels, desired.Annotations) // Update the service instance @@ -283,6 +289,9 @@ func (c *Client) updateUserProvided(ctx context.Context, observed *resource.Serv if creds != nil { upd.WithCredentials(creds) } + + upd.WithTags(toTagSlice(desired.Tags)) + upd.WithRouteServiceURL(desired.RouteServiceURL). WithSyslogDrainURL(desired.SyslogDrainURL) @@ -320,6 +329,9 @@ func UpdateObservation(in *v1alpha1.ServiceInstanceObservation, r *resource.Serv } in.ID = &r.GUID + + in.Tags = toTagPtrSlice(r.Tags) + in.LastOperation = v1alpha1.LastOperation{ Type: r.LastOperation.Type, State: r.LastOperation.State, @@ -344,6 +356,10 @@ func specUpToDate(in *v1alpha1.ServiceInstanceParameters, observed *resource.Ser return false } + if !tagsUpToDate(in.Tags, observed.Tags) { + return false + } + switch in.Type { case v1alpha1.ManagedService: if in.ServicePlan != nil && in.ServicePlan.ID != nil && observed.Relationships.ServicePlan.Data.GUID != *in.ServicePlan.ID { @@ -440,6 +456,7 @@ func getDesiredSharedSpaces(refs []v1alpha1.SpaceReference) []string { return guids } +// REVISE: This can be simplified using the sets k8s helper, like in tagsToUpdate // diffSharedSpaces compares the current and desired shared spaces and returns the spaces to add and remove to match the desired state func diffSharedSpaces(current, desired []string) (toAdd, toRemove []string) { currentSet := make(map[string]struct{}, len(current)) @@ -466,3 +483,28 @@ func diffSharedSpaces(current, desired []string) (toAdd, toRemove []string) { return toAdd, toRemove } + +func toTagSlice(in []*string) []string { + out := make([]string, 0, len(in)) + for _, t := range in { + if t != nil { + out = append(out, *t) + } + } + return out +} + +func toTagPtrSlice(in []string) []*string { + if len(in) == 0 { + return nil + } + out := make([]*string, len(in)) + for i := range in { + out[i] = &in[i] + } + return out +} + +func tagsUpToDate(desired []*string, observed []string) bool { + return sets.New(toTagSlice(desired)...).Equal(sets.New(observed...)) +} diff --git a/internal/clients/serviceinstance/serviceinstance_test.go b/internal/clients/serviceinstance/serviceinstance_test.go index 79c05df5..4c4e3cdd 100644 --- a/internal/clients/serviceinstance/serviceinstance_test.go +++ b/internal/clients/serviceinstance/serviceinstance_test.go @@ -568,6 +568,84 @@ func TestIsUpToDate_Metadata(t *testing.T) { }, want: false, }, + "Tags match same order": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: []*string{ptr.To("tag1"), ptr.To("tag2"), ptr.To("tag3")}, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{"tag1", "tag2", "tag3"}, + }, + want: true, + }, + "Tags match different order": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: []*string{ptr.To("tag2"), ptr.To("tag3"), ptr.To("tag1")}, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{"tag1", "tag2", "tag3"}, + }, + want: true, + }, + "Tags drift nil vs non-empty": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: nil, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{"tag"}, + }, + want: false, + }, + "Tags match nil vs empty": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: nil, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{}, + }, + want: true, + }, + "Tags match empty tags": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: []*string{}, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{}, + }, + want: true, + }, + "Tags drift empty vs observed non-empty": { + in: &v1alpha1.ServiceInstanceParameters{ + Name: ptr.To("name"), + Type: v1alpha1.ManagedService, + Tags: []*string{}, + }, + observed: &resource.ServiceInstance{ + Name: "name", + Type: "managed", + Tags: []string{"tag"}, + }, + want: false, + }, } for n, tc := range cases { From 0fdfcde45445f812798d40a360ce9264e2ae7379 Mon Sep 17 00:00:00 2001 From: Dambach Date: Wed, 30 Sep 2026 16:55:18 +0200 Subject: [PATCH 2/3] Fix complexity --- internal/clients/serviceinstance/serviceinstance.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/clients/serviceinstance/serviceinstance.go b/internal/clients/serviceinstance/serviceinstance.go index 0190ec41..92233ae3 100644 --- a/internal/clients/serviceinstance/serviceinstance.go +++ b/internal/clients/serviceinstance/serviceinstance.go @@ -360,6 +360,12 @@ func specUpToDate(in *v1alpha1.ServiceInstanceParameters, observed *resource.Ser return false } + return typeSpecUpToDate(in, observed) +} + +// typeSpecUpToDate checks the type-specific spec fields (service plan for managed, +// route service and syslog drain URLs for user-provided) against the observed CF resource. +func typeSpecUpToDate(in *v1alpha1.ServiceInstanceParameters, observed *resource.ServiceInstance) bool { switch in.Type { case v1alpha1.ManagedService: if in.ServicePlan != nil && in.ServicePlan.ID != nil && observed.Relationships.ServicePlan.Data.GUID != *in.ServicePlan.ID { From e693287752e7b0a67a6fa0335ccdf82ef54e3b67 Mon Sep 17 00:00:00 2001 From: Dambach Date: Mon, 5 Oct 2026 16:21:38 +0200 Subject: [PATCH 3/3] Update tags in userprovided and fix referenced function --- internal/clients/serviceinstance/serviceinstance.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/clients/serviceinstance/serviceinstance.go b/internal/clients/serviceinstance/serviceinstance.go index 92233ae3..17d9005b 100644 --- a/internal/clients/serviceinstance/serviceinstance.go +++ b/internal/clients/serviceinstance/serviceinstance.go @@ -206,7 +206,6 @@ func (c *Client) createUserProvided(ctx context.Context, mg xpresource.Managed, // create the service instance opt := resource.NewServiceInstanceCreateUserProvided(*spec.Name, *spec.Space) opt.Metadata = metadata.BuildMetadata(mg, spec.Labels, spec.Annotations) - opt.WithTags(toTagSlice(spec.Tags)) si, err := c.CreateUserProvided(ctx, opt) if err != nil { @@ -218,7 +217,8 @@ func (c *Client) createUserProvided(ctx context.Context, mg xpresource.Managed, if creds != nil { upt.WithCredentials(creds) } - upt.WithRouteServiceURL(spec.RouteServiceURL). + upt.WithTags(toTagSlice(spec.Tags)). + WithRouteServiceURL(spec.RouteServiceURL). WithSyslogDrainURL(spec.SyslogDrainURL) return c.UpdateUserProvided(ctx, si.GUID, upt) @@ -462,7 +462,7 @@ func getDesiredSharedSpaces(refs []v1alpha1.SpaceReference) []string { return guids } -// REVISE: This can be simplified using the sets k8s helper, like in tagsToUpdate +// REVISE: This can be simplified using the sets k8s helper, like in tagsUpToDate // diffSharedSpaces compares the current and desired shared spaces and returns the spaces to add and remove to match the desired state func diffSharedSpaces(current, desired []string) (toAdd, toRemove []string) { currentSet := make(map[string]struct{}, len(current))