diff --git a/mocks/pkg/types/service_controller.go b/mocks/pkg/types/service_controller.go index 9714355..6113489 100644 --- a/mocks/pkg/types/service_controller.go +++ b/mocks/pkg/types/service_controller.go @@ -104,9 +104,9 @@ func (_m *ServiceController) GetResourceManagerFactories() map[string]types.AWSR return r0 } -// NewAWSConfig provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4 -func (_m *ServiceController) NewAWSConfig(_a0 context.Context, _a1 v1alpha1.AWSRegion, _a2 *string, _a3 v1alpha1.AWSResourceName, _a4 schema.GroupVersionKind) (aws.Config, error) { - ret := _m.Called(_a0, _a1, _a2, _a3, _a4) +// NewAWSConfig provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4, _a5 +func (_m *ServiceController) NewAWSConfig(_a0 context.Context, _a1 v1alpha1.AWSRegion, _a2 *string, _a3 v1alpha1.AWSResourceName, _a4 schema.GroupVersionKind, _a5 map[string]string) (aws.Config, error) { + ret := _m.Called(_a0, _a1, _a2, _a3, _a4, _a5) if len(ret) == 0 { panic("no return value specified for NewAWSConfig") @@ -114,17 +114,17 @@ func (_m *ServiceController) NewAWSConfig(_a0 context.Context, _a1 v1alpha1.AWSR var r0 aws.Config var r1 error - if rf, ok := ret.Get(0).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind) (aws.Config, error)); ok { - return rf(_a0, _a1, _a2, _a3, _a4) + if rf, ok := ret.Get(0).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind, map[string]string) (aws.Config, error)); ok { + return rf(_a0, _a1, _a2, _a3, _a4, _a5) } - if rf, ok := ret.Get(0).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind) aws.Config); ok { - r0 = rf(_a0, _a1, _a2, _a3, _a4) + if rf, ok := ret.Get(0).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind, map[string]string) aws.Config); ok { + r0 = rf(_a0, _a1, _a2, _a3, _a4, _a5) } else { r0 = ret.Get(0).(aws.Config) } - if rf, ok := ret.Get(1).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind) error); ok { - r1 = rf(_a0, _a1, _a2, _a3, _a4) + if rf, ok := ret.Get(1).(func(context.Context, v1alpha1.AWSRegion, *string, v1alpha1.AWSResourceName, schema.GroupVersionKind, map[string]string) error); ok { + r1 = rf(_a0, _a1, _a2, _a3, _a4, _a5) } else { r1 = ret.Error(1) } diff --git a/pkg/runtime/config.go b/pkg/runtime/config.go index 82ad3a2..deaae6b 100644 --- a/pkg/runtime/config.go +++ b/pkg/runtime/config.go @@ -47,15 +47,28 @@ func (c *serviceController) NewAWSConfig( endpointURL *string, roleARN ackv1alpha1.AWSResourceName, groupVersionKind schema.GroupVersionKind, + labels map[string]string, ) (aws.Config, error) { + extra := []string{ + "GitCommit/" + c.VersionInfo.GitCommit, + "BuildDate/" + c.VersionInfo.BuildDate, + "CRDKind/" + groupVersionKind.Kind, + "CRDVersion/" + groupVersionKind.Version, + } + + // Add kro managed info if managed by kro + if isKROManaged(labels) { + extra = append(extra, "ManagedBy/kro") + if kroVersion := getKROVersion(labels); kroVersion != "" { + extra = append(extra, "KROVersion/"+kroVersion) + } + } + val := formatUserAgent( appName, groupVersionKind.Group+"-"+c.VersionInfo.GitVersion, - "GitCommit/"+c.VersionInfo.GitCommit, - "BuildDate/"+c.VersionInfo.BuildDate, - "CRDKind/"+groupVersionKind.Kind, - "CRDVersion/"+groupVersionKind.Version, + extra..., ) client := &clientWithUserAgent{ diff --git a/pkg/runtime/config_test.go b/pkg/runtime/config_test.go new file mode 100644 index 0000000..f7b71f6 --- /dev/null +++ b/pkg/runtime/config_test.go @@ -0,0 +1,234 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package runtime + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFormatUserAgent(t *testing.T) { + tests := []struct { + name string + appName string + version string + extra []string + expected string + }{ + { + name: "basic user agent without extras", + appName: "aws-controllers-k8s", + version: "s3-v1.2.3", + extra: nil, + expected: "aws-controllers-k8s/s3-v1.2.3", + }, + { + name: "user agent with single extra", + appName: "aws-controllers-k8s", + version: "s3-v1.2.3", + extra: []string{"GitCommit/abc123"}, + expected: "aws-controllers-k8s/s3-v1.2.3 (GitCommit/abc123)", + }, + { + name: "user agent with multiple extras", + appName: "aws-controllers-k8s", + version: "dynamodb-v1.2.3", + extra: []string{ + "GitCommit/abc123", + "BuildDate/2024-01-01", + "CRDKind/Table", + "CRDVersion/v1alpha1", + }, + expected: "aws-controllers-k8s/dynamodb-v1.2.3 (GitCommit/abc123; BuildDate/2024-01-01; CRDKind/Table; CRDVersion/v1alpha1)", + }, + { + name: "user agent with kro managed info", + appName: "aws-controllers-k8s", + version: "s3-v1.2.3", + extra: []string{ + "GitCommit/abc123", + "BuildDate/2024-01-01", + "CRDKind/Bucket", + "CRDVersion/v1alpha1", + "ManagedBy/kro", + "KROVersion/v0.1.0", + }, + expected: "aws-controllers-k8s/s3-v1.2.3 (GitCommit/abc123; BuildDate/2024-01-01; CRDKind/Bucket; CRDVersion/v1alpha1; ManagedBy/kro; KROVersion/v0.1.0)", + }, + { + name: "user agent with kro managed but no version", + appName: "aws-controllers-k8s", + version: "s3-v1.2.3", + extra: []string{ + "GitCommit/abc123", + "BuildDate/2024-01-01", + "CRDKind/Bucket", + "CRDVersion/v1alpha1", + "ManagedBy/kro", + }, + expected: "aws-controllers-k8s/s3-v1.2.3 (GitCommit/abc123; BuildDate/2024-01-01; CRDKind/Bucket; CRDVersion/v1alpha1; ManagedBy/kro)", + }, + { + name: "empty extra slice", + appName: "test-app", + version: "v1.0.0", + extra: []string{}, + expected: "test-app/v1.0.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := formatUserAgent(tt.appName, tt.version, tt.extra...) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsKROManaged(t *testing.T) { + tests := []struct { + name string + labels map[string]string + expected bool + }{ + { + name: "managed by kro", + labels: map[string]string{ + LabelManagedBy: "kro", + }, + expected: true, + }, + { + name: "managed by kro with other labels", + labels: map[string]string{ + "app": "myapp", + LabelManagedBy: "kro", + "env": "prod", + }, + expected: true, + }, + { + name: "managed by different controller", + labels: map[string]string{ + LabelManagedBy: "helm", + }, + expected: false, + }, + { + name: "managed-by label not present", + labels: map[string]string{ + "app": "myapp", + "env": "prod", + }, + expected: false, + }, + { + name: "nil labels", + labels: nil, + expected: false, + }, + { + name: "empty labels", + labels: map[string]string{}, + expected: false, + }, + { + name: "legacy kro.run/owned label (backward compatibility)", + labels: map[string]string{ + LabelKroOwned: "true", + }, + expected: true, + }, + { + name: "legacy kro.run/owned false", + labels: map[string]string{ + LabelKroOwned: "false", + }, + expected: false, + }, + { + name: "standard label takes precedence over legacy", + labels: map[string]string{ + LabelManagedBy: "kro", + LabelKroOwned: "false", + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isKROManaged(tt.labels) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetKROVersion(t *testing.T) { + tests := []struct { + name string + labels map[string]string + expected string + }{ + { + name: "kro version present", + labels: map[string]string{ + LabelKroVersion: "v0.1.0", + }, + expected: "v0.1.0", + }, + { + name: "kro version with other labels", + labels: map[string]string{ + "app": "myapp", + LabelKroVersion: "v1.2.3", + "env": "prod", + }, + expected: "v1.2.3", + }, + { + name: "kro version not present", + labels: map[string]string{ + "app": "myapp", + "env": "prod", + }, + expected: "", + }, + { + name: "nil labels", + labels: nil, + expected: "", + }, + { + name: "empty labels", + labels: map[string]string{}, + expected: "", + }, + { + name: "kro version with empty value", + labels: map[string]string{ + LabelKroVersion: "", + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getKROVersion(tt.labels) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 7ac078d..8e263f2 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -335,7 +335,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) // The config pivot to the roleARN will happen if it is not empty. // in the NewResourceManager - clientConfig, err := r.sc.NewAWSConfig(ctx, region, &endpointURL, roleARN, gvk) + clientConfig, err := r.sc.NewAWSConfig(ctx, region, &endpointURL, roleARN, gvk, desired.MetaObject().GetLabels()) if err != nil { return ctrlrt.Result{}, err } @@ -1173,6 +1173,7 @@ func (r *resourceReconciler) setResourceManaged( rlog.Debug("marked resource as managed") return nil } + // setResourceManagedAndAdopted marks the underlying CR in the supplied AWSResource with // a finalizer and adopted annotation that indicates the object is under ACK management and will not // be deleted until that finalizer is removed (in setResourceUnmanaged()) diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 246bd26..36db708 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -1813,6 +1813,7 @@ func TestReconcile_AccountDrifted(t *testing.T) { mock.Anything, mock.AnythingOfType("v1alpha1.AWSResourceName"), mock.AnythingOfType("schema.GroupVersionKind"), + mock.Anything, // labels map[string]string ).Return(aws.Config{}, nil) // Get fakeLogger diff --git a/pkg/runtime/tags.go b/pkg/runtime/tags.go index 21882b8..82f8aad 100644 --- a/pkg/runtime/tags.go +++ b/pkg/runtime/tags.go @@ -83,6 +83,20 @@ var ACKResourceTagFormats = map[string]resolveTagFormat{ gvk := obj.GetObjectKind().GroupVersionKind() return gvk.Kind }, + + acktags.ManagedByTagFormat: func( + obj rtclient.Object, + md acktypes.ServiceControllerMetadata, + ) string { + return getManagedBy(obj.GetLabels()) + }, + + acktags.KROVersionTagFormat: func( + obj rtclient.Object, + md acktypes.ServiceControllerMetadata, + ) string { + return getKROVersion(obj.GetLabels()) + }, } // GetDefaultTags provides Default tags (key value pairs) for given resource @@ -105,7 +119,11 @@ func GetDefaultTags( if key == "" || val == "" { continue } - defaultTags[key] = expandTagValue(val, obj, md) + expandedVal := expandTagValue(val, obj, md) + // Skip tags where the expanded value is empty + if expandedVal != "" { + defaultTags[key] = expandedVal + } } return defaultTags } diff --git a/pkg/runtime/tags_test.go b/pkg/runtime/tags_test.go index bc7dc0e..4047c94 100644 --- a/pkg/runtime/tags_test.go +++ b/pkg/runtime/tags_test.go @@ -18,8 +18,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime/schema" mocks "github.com/aws-controllers-k8s/runtime/mocks/controller-runtime/pkg/client" + schemaMocks "github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/runtime/schema" "github.com/aws-controllers-k8s/runtime/pkg/config" acktags "github.com/aws-controllers-k8s/runtime/pkg/tags" acktypes "github.com/aws-controllers-k8s/runtime/pkg/types" @@ -30,6 +32,14 @@ func TestGetDefaultTags(t *testing.T) { obj := mocks.Object{} obj.On("GetNamespace").Return("ns") obj.On("GetName").Return("res") + obj.On("GetLabels").Return(map[string]string{}) + + // Mock GetObjectKind to return a mock ObjectKind + mockObjectKind := &schemaMocks.ObjectKind{} + mockObjectKind.On("GroupVersionKind").Return(schema.GroupVersionKind{ + Kind: "Table", + }) + obj.On("GetObjectKind").Return(mockObjectKind) cfg := config.Config{} diff --git a/pkg/runtime/util.go b/pkg/runtime/util.go index 3e05e8c..81faca5 100644 --- a/pkg/runtime/util.go +++ b/pkg/runtime/util.go @@ -29,6 +29,17 @@ import ( // TODO(jaypipes): Place this code somewhere separate // (michaelhtm) ^ +1 +// (a-hilaly) ^ +1 + +// Label keys used by the runtime +const ( + // LabelManagedBy is the standard Kubernetes label for indicating which tool manages a resource + LabelManagedBy = "app.kubernetes.io/managed-by" + // LabelKroVersion is the kro-specific label for the kro version + LabelKroVersion = "kro.run/kro-version" + // LabelKroOwned is the legacy label for kro ownership (backward compatibility) + LabelKroOwned = "kro.run/owned" +) // AdoptionPolicy stores adoptionPolicy values we expect users to // provide in the resources `adoption-policy` annotation @@ -93,6 +104,80 @@ func IsReadOnly(res acktypes.AWSResource) bool { return false } +// IsManagedBy returns true if the supplied AWSResource has a label +// indicating that it is managed by the specified manager. +// It checks for the standard Kubernetes label app.kubernetes.io/managed-by. +func IsManagedBy(res acktypes.AWSResource, manager string) bool { + mo := res.MetaObject() + if mo == nil { + // Should never happen... if it does, it's buggy code. + panic("IsManagedBy received resource with nil RuntimeObject") + } + labels := mo.GetLabels() + if labels == nil { + return false + } + managedBy, exists := labels[LabelManagedBy] + return exists && managedBy == manager +} + +// KROVersion returns the kro version from the resource labels. +// Returns empty string if the kro.run/kro-version label is not present. +func KROVersion(res acktypes.AWSResource) string { + mo := res.MetaObject() + if mo == nil { + // Should never happen... if it does, it's buggy code. + panic("KROVersion received resource with nil RuntimeObject") + } + labels := mo.GetLabels() + if labels == nil { + return "" + } + version := labels[LabelKroVersion] + return version +} + +// isKROManaged returns true if the labels indicate the resource is managed by kro. +// It checks for the standard Kubernetes label app.kubernetes.io/managed-by set to "kro", +// and falls back to kro.run/owned for backward compatibility. +func isKROManaged(labels map[string]string) bool { + if labels == nil { + return false + } + + // Check standard Kubernetes label + if managedBy, exists := labels[LabelManagedBy]; exists && managedBy == "kro" { + return true + } + + // Check legacy label for backward compatibility + if owned, exists := labels[LabelKroOwned]; exists && owned == "true" { + return true + } + + return false +} + +// getManagedBy returns the value of the app.kubernetes.io/managed-by label. +// Returns empty string if the label is not present. +func getManagedBy(labels map[string]string) string { + if labels == nil { + return "" + } + managedBy, _ := labels[LabelManagedBy] + return managedBy +} + +// getKROVersion returns the kro version from the labels map. +// Returns empty string if the kro.run/kro-version label is not present. +func getKROVersion(labels map[string]string) string { + if labels == nil { + return "" + } + version := labels[LabelKroVersion] + return version +} + // GetAdoptionPolicy returns the Adoption Policy of the resource // defined by the user in annotation. Possible values are: // adopt-only | adopt-or-create @@ -197,7 +282,7 @@ func patchWithRetry( key := client.ObjectKeyFromObject(obj) freshObject := obj.DeepCopyObject().(client.Object) - err := apiReader.Get(ctx, key, freshObject) + err := apiReader.Get(ctx, key, freshObject) if err != nil { logger.Info(fmt.Sprintf("failed to refresh resource version during %s patch retry", operationType), "attempt", attempt, diff --git a/pkg/tags/tag_format.go b/pkg/tags/tag_format.go index 22ac50a..623977d 100644 --- a/pkg/tags/tag_format.go +++ b/pkg/tags/tag_format.go @@ -19,4 +19,6 @@ const ( NamespaceTagFormat = "%K8S_NAMESPACE%" ResourceNameTagFormat = "%K8S_RESOURCE_NAME%" ResourceKindTagFormat = "%K8S_RESOURCE_KIND%" + ManagedByTagFormat = "%MANAGED_BY%" + KROVersionTagFormat = "%KRO_VERSION%" ) diff --git a/pkg/types/service_controller.go b/pkg/types/service_controller.go index d8ed9d5..4dc088c 100644 --- a/pkg/types/service_controller.go +++ b/pkg/types/service_controller.go @@ -85,6 +85,7 @@ type ServiceController interface { *string, ackv1alpha1.AWSResourceName, schema.GroupVersionKind, + map[string]string, ) (aws.Config, error) // GetMetadata returns the metadata associated with the service controller.