From 524a7a519ff1f4fe139011659807ed020028badb Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:30:25 +1000 Subject: [PATCH] chore: centralise arn splitting --- spinifex/arn/acm.go | 6 +- spinifex/arn/arn.go | 14 +++++ spinifex/arn/arn_test.go | 81 +++++++++++++++++++++++++++ spinifex/arn/elbv2.go | 8 +-- spinifex/gateway/ecrapi/authz.go | 7 ++- spinifex/handlers/ecs/tags.go | 21 +++---- spinifex/handlers/eks/service_impl.go | 20 ++----- spinifex/handlers/sts/assume_role.go | 9 +-- 8 files changed, 128 insertions(+), 38 deletions(-) create mode 100644 spinifex/arn/arn.go create mode 100644 spinifex/arn/arn_test.go diff --git a/spinifex/arn/acm.go b/spinifex/arn/acm.go index 60cfa0444..738f32b78 100644 --- a/spinifex/arn/acm.go +++ b/spinifex/arn/acm.go @@ -23,11 +23,11 @@ func FormatACMCertificate(region, accountID, id string) string { // The id keeps everything after the first "/". Truncating at the last one would // let a grant on certificate/ authorize a request naming certificate//. func ParseACMCertificateID(certARN string) (string, bool) { - parts := strings.SplitN(certARN, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[2] != "acm" { + service, _, _, resource, ok := Split(certARN) + if !ok || service != "acm" { return "", false } - kind, id, found := strings.Cut(parts[5], "/") + kind, id, found := strings.Cut(resource, "/") if !found || ACMResourceType(kind) != ACMCertificate || id == "" { return "", false } diff --git a/spinifex/arn/arn.go b/spinifex/arn/arn.go new file mode 100644 index 000000000..9f0c3c998 --- /dev/null +++ b/spinifex/arn/arn.go @@ -0,0 +1,14 @@ +package arn + +import "strings" + +// Split breaks an arn:aws ARN into its service, region, account and resource +// components. The resource keeps any further colons or slashes verbatim. ok is +// false for anything outside the aws partition, the only one this stack builds. +func Split(s string) (service, region, account, resource string, ok bool) { + parts := strings.SplitN(s, ":", 6) + if len(parts) != 6 || parts[0] != "arn" || parts[1] != "aws" { + return "", "", "", "", false + } + return parts[2], parts[3], parts[4], parts[5], true +} diff --git a/spinifex/arn/arn_test.go b/spinifex/arn/arn_test.go new file mode 100644 index 000000000..9bb5895c0 --- /dev/null +++ b/spinifex/arn/arn_test.go @@ -0,0 +1,81 @@ +package arn_test + +import ( + "testing" + + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/stretchr/testify/assert" +) + +func TestSplit(t *testing.T) { + tests := []struct { + name string + value string + service string + region string + account string + resource string + wantOK bool + }{ + { + name: "regional ARN", + value: "arn:aws:ecs:ap-southeast-2:123456789012:cluster/web", + service: "ecs", + region: "ap-southeast-2", + account: "123456789012", + resource: "cluster/web", + wantOK: true, + }, + { + name: "global ARN has an empty region", + value: "arn:aws:iam::123456789012:root", + service: "iam", + account: "123456789012", + resource: "root", + wantOK: true, + }, + { + name: "resource keeps its own colons", + value: "arn:aws:rds:ap-southeast-2:123456789012:snapshot:rds:orders-2026-09-30", + service: "rds", + region: "ap-southeast-2", + account: "123456789012", + resource: "snapshot:rds:orders-2026-09-30", + wantOK: true, + }, + { + name: "empty resource", + value: "arn:aws:ecs:ap-southeast-2:123456789012:", + service: "ecs", + region: "ap-southeast-2", + account: "123456789012", + wantOK: true, + }, + { + name: "another partition", + value: "arn:aws-cn:ecs:cn-north-1:123456789012:cluster/web", + }, + { + name: "not an ARN", + value: "urn:aws:ecs:ap-southeast-2:123456789012:cluster/web", + }, + { + name: "too few components", + value: "arn:aws:ecs:ap-southeast-2:cluster/web", + }, + { + name: "empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + service, region, account, resource, ok := arn.Split(tt.value) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.service, service) + assert.Equal(t, tt.region, region) + assert.Equal(t, tt.account, account) + assert.Equal(t, tt.resource, resource) + }) + } +} diff --git a/spinifex/arn/elbv2.go b/spinifex/arn/elbv2.go index eaa19fc12..589d168ff 100644 --- a/spinifex/arn/elbv2.go +++ b/spinifex/arn/elbv2.go @@ -70,18 +70,18 @@ type ParsedELBv2 struct { // arn:aws:elasticloadbalancing ARN naming one of the four resource types: an // ARN this stack never builds has no resource it can correctly stand for. func ParseELBv2(value string) (ParsedELBv2, bool) { - parts := strings.SplitN(value, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[1] != "aws" || parts[2] != "elasticloadbalancing" { + service, region, account, rest, ok := Split(value) + if !ok || service != "elasticloadbalancing" { return ParsedELBv2{}, false } - rawKind, resource, found := strings.Cut(parts[5], "/") + rawKind, resource, found := strings.Cut(rest, "/") if !found || rawKind == "" { return ParsedELBv2{}, false } kind := ELBv2ResourceType(rawKind) switch kind { case ELBv2LoadBalancer, ELBv2TargetGroup, ELBv2Listener, ELBv2ListenerRule: - return ParsedELBv2{Region: parts[3], AccountID: parts[4], Kind: kind, Resource: resource}, true + return ParsedELBv2{Region: region, AccountID: account, Kind: kind, Resource: resource}, true default: return ParsedELBv2{}, false } diff --git a/spinifex/gateway/ecrapi/authz.go b/spinifex/gateway/ecrapi/authz.go index a7848d5a2..7930d60fb 100644 --- a/spinifex/gateway/ecrapi/authz.go +++ b/spinifex/gateway/ecrapi/authz.go @@ -8,6 +8,7 @@ import ( "slices" "strings" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awsec2query" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/gateway/bodyscope" @@ -214,11 +215,11 @@ func tagARN(region, accountID, resourceARN string) string { // resource ARN (arn:aws:ecr:::repository/). tagARN and // the tag handlers share this one parser for that shape. func RepositoryNameFromResourceARN(resourceARN string) (string, error) { - parts := strings.SplitN(resourceARN, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[2] != "ecr" { + service, _, _, resource, ok := arn.Split(resourceARN) + if !ok || service != "ecr" { return "", invalidResourceARNError() } - kind, name, found := strings.Cut(parts[5], "/") + kind, name, found := strings.Cut(resource, "/") if !found || kind != repositoryResourceType || name == "" { return "", invalidResourceARNError() } diff --git a/spinifex/handlers/ecs/tags.go b/spinifex/handlers/ecs/tags.go index 088ad57bf..ffd3ce4f0 100644 --- a/spinifex/handlers/ecs/tags.go +++ b/spinifex/handlers/ecs/tags.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/nats-io/nats.go/jetstream" ) @@ -37,22 +38,22 @@ func ResourceARNSegment(resourceARN string) (string, error) { // (.../cluster/{name}, .../task-definition/{family}:{rev}); // service/task/container-instance ARNs embed the owning cluster. func parseResourceARN(resourceARN string) (kind ecsResourceKind, cluster, id, segment string, err error) { - // SplitN(..., 6) keeps the resource segment intact even though a - // task-definition id ("family:1") itself contains a colon. - parts := strings.SplitN(resourceARN, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[2] != "ecs" { + // Split keeps the resource segment intact even though a task-definition + // id ("family:1") itself contains a colon. + service, _, _, segment, ok := arn.Split(resourceARN) + if !ok || service != "ecs" { return 0, "", "", "", errors.New(awserrors.ErrorECSInvalidParameter) } - rtype, rest, ok := strings.Cut(parts[5], "/") + rtype, rest, ok := strings.Cut(segment, "/") if !ok || rtype == "" || rest == "" { return 0, "", "", "", errors.New(awserrors.ErrorECSInvalidParameter) } switch rtype { case "cluster": - return ecsResourceCluster, rest, rest, parts[5], nil + return ecsResourceCluster, rest, rest, segment, nil case "task-definition": - return ecsResourceTaskDefinition, "", rest, parts[5], nil + return ecsResourceTaskDefinition, "", rest, segment, nil case "service", "task", "container-instance": clusterName, name, ok := strings.Cut(rest, "/") if !ok || clusterName == "" || name == "" { @@ -60,11 +61,11 @@ func parseResourceARN(resourceARN string) (kind ecsResourceKind, cluster, id, se } switch rtype { case "service": - return ecsResourceService, clusterName, name, parts[5], nil + return ecsResourceService, clusterName, name, segment, nil case "task": - return ecsResourceTask, clusterName, name, parts[5], nil + return ecsResourceTask, clusterName, name, segment, nil default: - return ecsResourceContainerInstance, clusterName, name, parts[5], nil + return ecsResourceContainerInstance, clusterName, name, segment, nil } default: return 0, "", "", "", errors.New(awserrors.ErrorECSInvalidParameter) diff --git a/spinifex/handlers/eks/service_impl.go b/spinifex/handlers/eks/service_impl.go index fabd715c2..5fd83caf2 100644 --- a/spinifex/handlers/eks/service_impl.go +++ b/spinifex/handlers/eks/service_impl.go @@ -2001,15 +2001,11 @@ func (s *EKSServiceImpl) accountBucket(ctx context.Context, accountID string) (j // (arn:aws:eks:::cluster/), reporting false for any other // ARN shape (e.g. nodegroup) the tag store does not back. func clusterNameFromARN(arn string) (string, bool) { - const prefix = "arn:aws:eks:" - if !strings.HasPrefix(arn, prefix) { + service, _, _, resource, ok := resourcearn.Split(arn) + if !ok || service != "eks" { return "", false } - parts := strings.SplitN(arn, ":", 6) - if len(parts) != 6 { - return "", false - } - resType, name, found := strings.Cut(parts[5], "/") + resType, name, found := strings.Cut(resource, "/") if !found || resType != "cluster" || name == "" { return "", false } @@ -2020,15 +2016,11 @@ func clusterNameFromARN(arn string) (string, bool) { // (arn:aws:eks:::nodegroup///), reporting false // for any other ARN shape. func nodegroupRefFromARN(arn string) (string, string, bool) { - const prefix = "arn:aws:eks:" - if !strings.HasPrefix(arn, prefix) { + service, _, _, resource, ok := resourcearn.Split(arn) + if !ok || service != "eks" { return "", "", false } - parts := strings.SplitN(arn, ":", 6) - if len(parts) != 6 { - return "", "", false - } - resType, rest, found := strings.Cut(parts[5], "/") + resType, rest, found := strings.Cut(resource, "/") if !found || resType != "nodegroup" { return "", "", false } diff --git a/spinifex/handlers/sts/assume_role.go b/spinifex/handlers/sts/assume_role.go index 74b6e17bb..b426b66ba 100644 --- a/spinifex/handlers/sts/assume_role.go +++ b/spinifex/handlers/sts/assume_role.go @@ -20,6 +20,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/sts" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" "github.com/mulgadc/spinifex/spinifex/kvstore" @@ -438,7 +439,7 @@ func matchAWSPrincipalEntry(clause, callerARN string) bool { return true } if utils.IsAccountID(clause) { - clause = fmt.Sprintf("arn:aws:iam::%s:root", clause) + clause = arn.FormatIAMRoot(clause) } if clause == callerARN { return true @@ -485,11 +486,11 @@ type principalARN struct { } func parsePrincipalARN(arnStr string) (principalARN, bool) { - parts := strings.SplitN(arnStr, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[1] != "aws" || parts[3] != "" { + service, region, account, resource, ok := arn.Split(arnStr) + if !ok || region != "" { return principalARN{}, false } - return principalARN{service: parts[2], account: parts[4], resource: parts[5]}, true + return principalARN{service: service, account: account, resource: resource}, true } func lastPathSegment(s string) string {