From cba1ecceaf942bc387c4b072e4f491d31248a139 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Tue, 16 Jun 2026 16:37:44 +1000 Subject: [PATCH] fix: route EKS DisassociateAccessPolicy policyArn path segment (#367) The DisassociateAccessPolicy route regex terminated at /access-entries/{principalArn}/access-policies, but the AWS EKS REST API sends the policy ARN as the final path segment: DELETE /clusters/{name}/access-entries/{principalArn}/access-policies/{policyArn} The route never matched, so the gateway returned InvalidActionException (seen as a 400 on 'tofu destroy' of aws_eks_access_policy_association). The handler also read policyArn from the request body, which AWS leaves empty for this DELETE. Capture policyArn as a third path param and pass it through to the input, dropping the body read. Update route and wrapper tests accordingly. --- spinifex/gateway/eks.go | 4 ++-- spinifex/gateway/eks/access.go | 13 ++++++------- spinifex/gateway/eks/wrappers_test.go | 8 +------- spinifex/gateway/eks_test.go | 21 +++++++++++++++++++-- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/spinifex/gateway/eks.go b/spinifex/gateway/eks.go index 6447365b6..2ebb0f98f 100644 --- a/spinifex/gateway/eks.go +++ b/spinifex/gateway/eks.go @@ -112,9 +112,9 @@ var eksRoutes = []eksRoute{ func(gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { return gateway_eks.AssociateAccessPolicy(gw.NATSConn, acct, p[0], p[1], b) }}, - {"DELETE", regexp.MustCompile(`^/clusters/([^/]+)/access-entries/([^/]+)/access-policies$`), "DisassociateAccessPolicy", + {"DELETE", regexp.MustCompile(`^/clusters/([^/]+)/access-entries/([^/]+)/access-policies/([^/]+)$`), "DisassociateAccessPolicy", func(gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { - return gateway_eks.DisassociateAccessPolicy(gw.NATSConn, acct, p[0], p[1], b) + return gateway_eks.DisassociateAccessPolicy(gw.NATSConn, acct, p[0], p[1], p[2]) }}, {"GET", regexp.MustCompile(`^/clusters/([^/]+)/access-entries/([^/]+)/access-policies$`), "ListAssociatedAccessPolicies", func(gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { diff --git a/spinifex/gateway/eks/access.go b/spinifex/gateway/eks/access.go index e3e416f0d..333ef5bab 100644 --- a/spinifex/gateway/eks/access.go +++ b/spinifex/gateway/eks/access.go @@ -63,14 +63,13 @@ func AssociateAccessPolicy(natsConn *nats.Conn, accountID, cluster, principalARN return handlers_eks.NewNATSEKSService(natsConn).AssociateAccessPolicy(input, accountID) } -// DisassociateAccessPolicy — DELETE /clusters/{name}/access-entries/{arn}/access-policies -func DisassociateAccessPolicy(natsConn *nats.Conn, accountID, cluster, principalARN string, body []byte) (*eks.DisassociateAccessPolicyOutput, error) { - input := new(eks.DisassociateAccessPolicyInput) - if err := unmarshalIfBody(body, input); err != nil { - return nil, err +// DisassociateAccessPolicy — DELETE /clusters/{name}/access-entries/{arn}/access-policies/{policyArn} +func DisassociateAccessPolicy(natsConn *nats.Conn, accountID, cluster, principalARN, policyARN string) (*eks.DisassociateAccessPolicyOutput, error) { + input := &eks.DisassociateAccessPolicyInput{ + ClusterName: aws.String(cluster), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), } - input.ClusterName = aws.String(cluster) - input.PrincipalArn = aws.String(principalARN) return handlers_eks.NewNATSEKSService(natsConn).DisassociateAccessPolicy(input, accountID) } diff --git a/spinifex/gateway/eks/wrappers_test.go b/spinifex/gateway/eks/wrappers_test.go index 00cf68ded..222907c07 100644 --- a/spinifex/gateway/eks/wrappers_test.go +++ b/spinifex/gateway/eks/wrappers_test.go @@ -172,14 +172,10 @@ func TestGatewayWrappers_Access(t *testing.T) { require.NoError(t, err) assert.NotNil(t, out9) - out10, err := DisassociateAccessPolicy(nc, acct, "alpha", "arn-user", []byte(`{}`)) + out10, err := DisassociateAccessPolicy(nc, acct, "alpha", "arn-user", "policy-arn") require.NoError(t, err) assert.NotNil(t, out10) - out11, err := DisassociateAccessPolicy(nc, acct, "alpha", "arn-user", nil) - require.NoError(t, err) - assert.NotNil(t, out11) - out12, err := ListAssociatedAccessPolicies(nc, acct, "alpha", "arn-user") require.NoError(t, err) assert.NotNil(t, out12) @@ -199,8 +195,6 @@ func TestGatewayWrappers_Access_BadJSON(t *testing.T) { require.Error(t, err) _, err = AssociateAccessPolicy(nc, acct, "alpha", "arn-user", []byte(`{x`)) require.Error(t, err) - _, err = DisassociateAccessPolicy(nc, acct, "alpha", "arn-user", []byte(`{x`)) - require.Error(t, err) } func TestGatewayWrappers_Addons(t *testing.T) { diff --git a/spinifex/gateway/eks_test.go b/spinifex/gateway/eks_test.go index 811e26c28..351a0623f 100644 --- a/spinifex/gateway/eks_test.go +++ b/spinifex/gateway/eks_test.go @@ -38,7 +38,7 @@ func TestLookupEKSAction_ResolvesKnownRoutes(t *testing.T) { {"POST", "/clusters/alpha/access-entries/arn-user", "UpdateAccessEntry", []string{"alpha", "arn-user"}}, {"DELETE", "/clusters/alpha/access-entries/arn-user", "DeleteAccessEntry", []string{"alpha", "arn-user"}}, {"POST", "/clusters/alpha/access-entries/arn-user/access-policies", "AssociateAccessPolicy", []string{"alpha", "arn-user"}}, - {"DELETE", "/clusters/alpha/access-entries/arn-user/access-policies", "DisassociateAccessPolicy", []string{"alpha", "arn-user"}}, + {"DELETE", "/clusters/alpha/access-entries/arn-user/access-policies/policy-arn", "DisassociateAccessPolicy", []string{"alpha", "arn-user", "policy-arn"}}, {"GET", "/clusters/alpha/access-entries/arn-user/access-policies", "ListAssociatedAccessPolicies", []string{"alpha", "arn-user"}}, {"GET", "/access-policies", "ListAccessPolicies", nil}, {"GET", "/clusters/alpha/addons", "ListAddons", []string{"alpha"}}, @@ -81,7 +81,6 @@ func TestLookupEKSAction_EncodedPrincipalARN(t *testing.T) { {"POST", "/clusters/alpha/access-entries/" + escaped, "UpdateAccessEntry"}, {"DELETE", "/clusters/alpha/access-entries/" + escaped, "DeleteAccessEntry"}, {"POST", "/clusters/alpha/access-entries/" + escaped + "/access-policies", "AssociateAccessPolicy"}, - {"DELETE", "/clusters/alpha/access-entries/" + escaped + "/access-policies", "DisassociateAccessPolicy"}, {"GET", "/clusters/alpha/access-entries/" + escaped + "/access-policies", "ListAssociatedAccessPolicies"}, } for _, tc := range cases { @@ -95,6 +94,24 @@ func TestLookupEKSAction_EncodedPrincipalARN(t *testing.T) { } } +// DisassociateAccessPolicy carries the policy ARN as the final path segment +// (DELETE .../access-policies/{policyArn}); both principal and policy ARNs are +// percent-encoded and must survive as separate single segments. +func TestLookupEKSAction_DisassociateEncodedARNs(t *testing.T) { + const ( + principal = "arn:aws:iam::000000000001:user/admin" + principalEscaped = "arn%3Aaws%3Aiam%3A%3A000000000001%3Auser%2Fadmin" + policy = "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy" + policyEscaped = "arn%3Aaws%3Aeks%3A%3Aaws%3Acluster-access-policy%2FAmazonEKSViewPolicy" + ) + path := "/clusters/alpha/access-entries/" + principalEscaped + "/access-policies/" + policyEscaped + action, params, handler, ok := lookupEKSAction("DELETE", path) + require.True(t, ok, "encoded disassociate path should match: %s", path) + require.NotNil(t, handler) + assert.Equal(t, "DisassociateAccessPolicy", action) + assert.Equal(t, []string{"alpha", principal, policy}, params) +} + func TestLookupEKSAction_CoversAllActions(t *testing.T) { expected := map[string]bool{ "CreateCluster": false,