From 5fa3a2bc3a5efca6ea884f30e94dc039041d98c9 Mon Sep 17 00:00:00 2001 From: Amine Date: Sat, 22 Nov 2025 20:10:24 -0800 Subject: [PATCH] feat(runtime): add KRO context to user agent and tags When resources are managed by KRO, ACK now includes KRO information in AWS API calls through user agent strings and resource tags. This makes it easier to understand which resources came from KRO when looking at AWS CloudTrail logs, cost allocation reports, and usage metrics. The implementation detects KRO managed resources via the standard Kubernetes `app.kubernetes.io/managed-by` label (with backward compatibility for the legacy `kro.run/owned` label), and adds optional `%MANAGED_BY%` and `%KRO_VERSION%` tag formats that can be used in default tags configuration. Empty tag values are automatically skipped to keep resources clean. --- mocks/pkg/types/service_controller.go | 18 +- pkg/runtime/config.go | 21 ++- pkg/runtime/config_test.go | 234 ++++++++++++++++++++++++++ pkg/runtime/reconciler.go | 3 +- pkg/runtime/reconciler_test.go | 1 + pkg/runtime/tags.go | 20 ++- pkg/runtime/tags_test.go | 10 ++ pkg/runtime/util.go | 87 +++++++++- pkg/tags/tag_format.go | 2 + pkg/types/service_controller.go | 1 + 10 files changed, 381 insertions(+), 16 deletions(-) create mode 100644 pkg/runtime/config_test.go 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.