diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index fcbb56a..2d0a541 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,8 +1,8 @@ ack_generate_info: - build_date: "2025-07-22T21:50:37Z" - build_hash: b2dc0f44e0b08f041de14c3944a5cc005ba97c8f - go_version: go1.24.5 - version: v0.50.0 + build_date: "2025-07-30T23:41:55Z" + build_hash: 300e3ab143b837dda0dc00794c21b0edc1530c58 + go_version: go1.24.4 + version: v0.50.0-2-g300e3ab-dirty api_directory_checksum: 2108338a86d704419192e545c0bfb433bab8c836 api_version: v1alpha1 aws_sdk_go_version: v1.32.6 diff --git a/config/crd/bases/s3.services.k8s.aws_buckets.yaml b/config/crd/bases/s3.services.k8s.aws_buckets.yaml index e5e4990..8d806c7 100644 --- a/config/crd/bases/s3.services.k8s.aws_buckets.yaml +++ b/config/crd/bases/s3.services.k8s.aws_buckets.yaml @@ -1219,12 +1219,17 @@ spec: OwnerAccountID is the AWS Account ID of the account that owns the backend AWS service API resource. type: string + partition: + description: Partition is the AWS partition in which the resource + exists or will exist + type: string region: description: Region is the AWS region in which the resource exists or will exist. type: string required: - ownerAccountID + - partition - region type: object conditions: diff --git a/go.mod b/go.mod index edb8525..f2e2a00 100644 --- a/go.mod +++ b/go.mod @@ -94,3 +94,5 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.4.2 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) + +replace github.com/aws-controllers-k8s/runtime => github.com/michaelhtm/ack-runtime v0.49.1-0.20250730215011-f7ca6ab251ea diff --git a/go.sum b/go.sum index c9a125a..2691dbe 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/aws-controllers-k8s/runtime v0.50.0 h1:6BXOBdnb+xw6uSMEDeALhTKc4veZR9NfXIsl5QJKZ8k= -github.com/aws-controllers-k8s/runtime v0.50.0/go.mod h1:OkUJN+Ds799JLYZsMJrO2vDJ4snxUeHK2MgrQHbU+Qc= github.com/aws/aws-sdk-go v1.49.0 h1:g9BkW1fo9GqKfwg2+zCD+TW/D36Ux+vtfJ8guF4AYmY= github.com/aws/aws-sdk-go v1.49.0/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= github.com/aws/aws-sdk-go-v2 v1.34.0 h1:9iyL+cjifckRGEVpRKZP3eIxVlL06Qk1Tk13vreaVQU= @@ -117,6 +115,8 @@ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= +github.com/michaelhtm/ack-runtime v0.49.1-0.20250730215011-f7ca6ab251ea h1:hyMdoRxWLIS1ner6atIh2BeaKMxnRQhTXdzs2WDZBjs= +github.com/michaelhtm/ack-runtime v0.49.1-0.20250730215011-f7ca6ab251ea/go.mod h1:OkUJN+Ds799JLYZsMJrO2vDJ4snxUeHK2MgrQHbU+Qc= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= diff --git a/helm/crds/s3.services.k8s.aws_buckets.yaml b/helm/crds/s3.services.k8s.aws_buckets.yaml index 39edc94..20affe6 100644 --- a/helm/crds/s3.services.k8s.aws_buckets.yaml +++ b/helm/crds/s3.services.k8s.aws_buckets.yaml @@ -1219,12 +1219,17 @@ spec: OwnerAccountID is the AWS Account ID of the account that owns the backend AWS service API resource. type: string + partition: + description: Partition is the AWS partition in which the resource + exists or will exist + type: string region: description: Region is the AWS region in which the resource exists or will exist. type: string required: - ownerAccountID + - partition - region type: object conditions: diff --git a/pkg/resource/bucket/hook.go b/pkg/resource/bucket/hook.go index a423f58..6f6a23e 100644 --- a/pkg/resource/bucket/hook.go +++ b/pkg/resource/bucket/hook.go @@ -15,7 +15,6 @@ package bucket import ( "context" - "fmt" "strings" "github.com/pkg/errors" @@ -29,19 +28,6 @@ import ( svcsdktypes "github.com/aws/aws-sdk-go-v2/service/s3/types" ) -// bucketARN returns the ARN of the S3 bucket with the given name. -func bucketARN(bucketName string) string { - // TODO(a-hilaly): I know there could be other partitions, but I'm - // not sure how to determine at this level of abstraction. Probably - // something the SDK/runtime should handle. For now, we'll just use - // the `aws` partition. - // - // NOTE(a-hilaly): Other parts of ACK also use this default partition - // e.g the generated function `ARNFromName` also uses `aws` as the - // default partition. - return fmt.Sprintf("arn:aws:s3:::%s", bucketName) -} - var ( DefaultAccelerationStatus = svcsdktypes.BucketAccelerateStatusSuspended DefaultRequestPayer = svcsdktypes.PayerBucketOwner diff --git a/pkg/resource/bucket/identifiers.go b/pkg/resource/bucket/identifiers.go index 1025e5c..8e35261 100644 --- a/pkg/resource/bucket/identifiers.go +++ b/pkg/resource/bucket/identifiers.go @@ -53,3 +53,12 @@ func (ri *resourceIdentifiers) Region() *ackv1alpha1.AWSRegion { } return nil } + +// Partition returns the AWS partition in which the reosurce exists, or +// nil if this information is not known. +func (ri *resourceIdentifiers) Partition() *ackv1alpha1.AWSPartition { + if ri.meta != nil { + return ri.meta.Partition + } + return nil +} diff --git a/pkg/resource/bucket/manager.go b/pkg/resource/bucket/manager.go index 00adf0f..8a1671f 100644 --- a/pkg/resource/bucket/manager.go +++ b/pkg/resource/bucket/manager.go @@ -75,6 +75,8 @@ type resourceManager struct { awsAccountID ackv1alpha1.AWSAccountID // The AWS Region that this resource manager targets awsRegion ackv1alpha1.AWSRegion + // The AWS Partition that this resource manager targets + awsPartition ackv1alpha1.AWSPartition // sdk is a pointer to the AWS service API client exposed by the // aws-sdk-go-v2/services/{alias} package. sdkapi *svcsdk.Client @@ -193,7 +195,8 @@ func (rm *resourceManager) Delete( // name for the resource func (rm *resourceManager) ARNFromName(name string) string { return fmt.Sprintf( - "arn:aws:s3:%s:%s:%s", + "arn:%s:s3:%s:%s:%s", + rm.awsPartition, rm.awsRegion, rm.awsAccountID, name, @@ -368,6 +371,7 @@ func newResourceManager( rr acktypes.Reconciler, id ackv1alpha1.AWSAccountID, region ackv1alpha1.AWSRegion, + partition ackv1alpha1.AWSPartition, ) (*resourceManager, error) { return &resourceManager{ cfg: cfg, @@ -377,6 +381,7 @@ func newResourceManager( rr: rr, awsAccountID: id, awsRegion: region, + awsPartition: partition, sdkapi: svcsdk.NewFromConfig(clientcfg), }, nil } diff --git a/pkg/resource/bucket/manager_factory.go b/pkg/resource/bucket/manager_factory.go index 3b7532c..b01408d 100644 --- a/pkg/resource/bucket/manager_factory.go +++ b/pkg/resource/bucket/manager_factory.go @@ -43,18 +43,13 @@ func (f *resourceManagerFactory) ResourceDescriptor() acktypes.AWSResourceDescri return &resourceDescriptor{} } -// ManagerFor returns a resource manager object that can manage resources for a -// supplied AWS account -func (f *resourceManagerFactory) ManagerFor( - cfg ackcfg.Config, - clientcfg aws.Config, - log logr.Logger, - metrics *ackmetrics.Metrics, - rr acktypes.Reconciler, +// GetCachedManager returns a manager object that can manage resources for a +// supplied AWS account if it was already created and cached, or nil if not +func (f *resourceManagerFactory) GetCachedManager( id ackv1alpha1.AWSAccountID, region ackv1alpha1.AWSRegion, roleARN ackv1alpha1.AWSResourceName, -) (acktypes.AWSResourceManager, error) { +) acktypes.AWSResourceManager { // We use the account ID, region, and role ARN to uniquely identify a // resource manager. This helps us to avoid creating multiple resource // managers for the same account/region/roleARN combination. @@ -62,15 +57,34 @@ func (f *resourceManagerFactory) ManagerFor( f.RLock() rm, found := f.rmCache[rmId] f.RUnlock() - - if found { - return rm, nil + if !found { + return nil } + return rm +} + +// ManagerFor returns a resource manager object that can manage resources for a +// supplied AWS account +func (f *resourceManagerFactory) ManagerFor( + cfg ackcfg.Config, + clientcfg aws.Config, + log logr.Logger, + metrics *ackmetrics.Metrics, + rr acktypes.Reconciler, + id ackv1alpha1.AWSAccountID, + region ackv1alpha1.AWSRegion, + partition ackv1alpha1.AWSPartition, + roleARN ackv1alpha1.AWSResourceName, +) (acktypes.AWSResourceManager, error) { f.Lock() defer f.Unlock() - rm, err := newResourceManager(cfg, clientcfg, log, metrics, rr, id, region) + // We use the account ID, region, partition, and role ARN to uniquely identify a + // resource manager. This helps us to avoid creating multiple resource + // managers for the same account/region/roleARN combination. + rmId := fmt.Sprintf("%s/%s/%s", id, region, roleARN) + rm, err := newResourceManager(cfg, clientcfg, log, metrics, rr, id, region, partition) if err != nil { return nil, err } diff --git a/pkg/resource/bucket/sdk.go b/pkg/resource/bucket/sdk.go index 3028e3f..482fa0c 100644 --- a/pkg/resource/bucket/sdk.go +++ b/pkg/resource/bucket/sdk.go @@ -114,7 +114,7 @@ func (rm *resourceManager) sdkFind( } // Set bucket ARN in the output - bucketARN := ackv1alpha1.AWSResourceName(bucketARN(*ko.Spec.Name)) + bucketARN := ackv1alpha1.AWSResourceName(rm.ARNFromName(*ko.Spec.Name)) ko.Status.ACKResourceMetadata.ARN = &bucketARN return &resource{ko}, nil } @@ -291,6 +291,9 @@ func (rm *resourceManager) setStatusDefaults( if ko.Status.ACKResourceMetadata.Region == nil { ko.Status.ACKResourceMetadata.Region = &rm.awsRegion } + if ko.Status.ACKResourceMetadata.Partition == nil { + ko.Status.ACKResourceMetadata.Partition = &rm.awsPartition + } if ko.Status.ACKResourceMetadata.OwnerAccountID == nil { ko.Status.ACKResourceMetadata.OwnerAccountID = &rm.awsAccountID } diff --git a/templates/hooks/bucket/sdk_read_many_post_set_output.go.tpl b/templates/hooks/bucket/sdk_read_many_post_set_output.go.tpl index 73e3334..c505b55 100644 --- a/templates/hooks/bucket/sdk_read_many_post_set_output.go.tpl +++ b/templates/hooks/bucket/sdk_read_many_post_set_output.go.tpl @@ -3,5 +3,5 @@ } // Set bucket ARN in the output - bucketARN := ackv1alpha1.AWSResourceName(bucketARN(*ko.Spec.Name)) + bucketARN := ackv1alpha1.AWSResourceName(rm.ARNFromName(*ko.Spec.Name)) ko.Status.ACKResourceMetadata.ARN = &bucketARN \ No newline at end of file diff --git a/test/e2e/requirements.txt b/test/e2e/requirements.txt index 192b6b9..c623b7c 100644 --- a/test/e2e/requirements.txt +++ b/test/e2e/requirements.txt @@ -1 +1 @@ -acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@a49300708a1a5586fec36fd28a2494d9cb2288ab +acktest @ git+https://github.com/michaelhtm/ack-test-infra.git@38295c88b5b47c2c7cedd98ef2fc2f505f11551a diff --git a/test/e2e/tests/test_bucket.py b/test/e2e/tests/test_bucket.py index d192484..d6a7061 100644 --- a/test/e2e/tests/test_bucket.py +++ b/test/e2e/tests/test_bucket.py @@ -24,7 +24,7 @@ from acktest.resources import random_suffix_name from acktest.k8s import resource as k8s -from acktest.aws.identity import get_region +from acktest.aws.identity import get_region, get_account_id, get_partition from acktest import adoption as adoption from acktest import tags as tags from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_s3_resource @@ -116,9 +116,9 @@ def basic_bucket(s3_client) -> Generator[Bucket, None, None]: assert k8s.get_resource_exists(bucket.ref) # assert bucket ARN is present in status + # hardcoding partition for now as the test infra is local bucket_k8s = bucket.resource_data = k8s.get_resource(bucket.ref) - assert "arn:aws:s3:::" + bucket.resource_name == bucket_k8s["status"]["ackResourceMetadata"]["arn"] - + assert "arn:"+ get_partition() +":s3:" + get_region() + ":" + get_account_id() + ":" + bucket.resource_name == bucket_k8s["status"]["ackResourceMetadata"]["arn"] exists = bucket_exists(s3_client, bucket) assert exists except: @@ -344,4 +344,4 @@ def _update_assert_website(self, bucket: Bucket, s3_resource): latest = website assert desired["errorDocument"]["key"] == latest.error_document["Key"] - assert desired["indexDocument"]["suffix"] == latest.index_document["Suffix"] \ No newline at end of file + assert desired["indexDocument"]["suffix"] == latest.index_document["Suffix"]