From 0b93b1a9578694fb7090a7d2801ddad0dbfafcb4 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Fri, 18 Sep 2026 17:10:48 +1000 Subject: [PATCH] fix: route the EKS update reads so they refuse instead of reading as a typo (#1084) * fix: route the EKS update reads so they refuse instead of reading as a typo UpdateClusterVersion was registered at POST /clusters/{name}/update-version, which is the nodegroup pattern and not a path the SDK ever calls; AWS puts it at POST /clusters/{name}/updates. The refusal written for it was therefore unreachable, and the router answered InvalidAction, which tells a caller they typed something that does not exist. ListUpdates and DescribeUpdate had no route at all and answered the same way. Both now refuse with NotImplemented at the gateway rather than through a service method: the update surface they would read is absent, so a round-trip to reach a constant refusal is surface for its own sake. The absence of the cluster-level update surface is unchanged. This only stops the platform blaming the caller's spelling for its own routing. * test: account for the two EKS update reads in the cross-tenant table Both name their cluster by a path segment that carries no account segment, so neither can name another tenant's resource and neither needs a case of its own. --- spinifex/gateway/eks.go | 14 +++++++++++- spinifex/gateway/eks/authz.go | 2 ++ spinifex/gateway/eks_test.go | 30 +++++++++++++++++++++++++- tests/integration/cross_tenant_test.go | 4 ++-- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/spinifex/gateway/eks.go b/spinifex/gateway/eks.go index d9b64e682..74005cc1b 100644 --- a/spinifex/gateway/eks.go +++ b/spinifex/gateway/eks.go @@ -53,10 +53,22 @@ var eksRoutes = []eksRoute{ func(ctx context.Context, gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { return gateway_eks.UpdateClusterConfig(ctx, gw.NATSConn, acct, p[0], b) }}, - {"POST", "/clusters/{clusterName}/update-version", "UpdateClusterVersion", + {"POST", "/clusters/{clusterName}/updates", "UpdateClusterVersion", func(ctx context.Context, gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { return gateway_eks.UpdateClusterVersion(ctx, gw.NATSConn, acct, p[0], b) }}, + // The update surface these two read is absent, so they refuse here rather + // than through a service method: a round-trip to reach a constant refusal + // is surface for its own sake. Left unregistered they would answer + // InvalidAction, which blames the caller's spelling for a routing gap. + {"GET", "/clusters/{clusterName}/updates", "ListUpdates", + func(ctx context.Context, gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { + return nil, errors.New(awserrors.ErrorNotImplemented) + }}, + {"GET", "/clusters/{clusterName}/updates/{updateId}", "DescribeUpdate", + func(ctx context.Context, gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { + return nil, errors.New(awserrors.ErrorNotImplemented) + }}, // Control-plane VM broker: relays bootstrap/state POSTs onto eks.bus.*/eks.state.* NATS subjects. // acct and callerARN are ignored; cluster account comes from the body. {"POST", "/clusters/{clusterName}/internal-publish", "PublishInternal", diff --git a/spinifex/gateway/eks/authz.go b/spinifex/gateway/eks/authz.go index 05f915cf6..5aee13e87 100644 --- a/spinifex/gateway/eks/authz.go +++ b/spinifex/gateway/eks/authz.go @@ -49,6 +49,8 @@ var eksScopes = map[string][]resourceSource{ "DeleteCluster": {sourceCluster}, "UpdateClusterConfig": {sourceCluster}, "UpdateClusterVersion": {sourceCluster}, + "ListUpdates": {sourceCluster}, + "DescribeUpdate": {sourceCluster}, "ListClusters": {sourceAny}, // Internal control-plane routes. The cluster's owning account is a path diff --git a/spinifex/gateway/eks_test.go b/spinifex/gateway/eks_test.go index e374ba6b7..518fba63c 100644 --- a/spinifex/gateway/eks_test.go +++ b/spinifex/gateway/eks_test.go @@ -25,7 +25,9 @@ func TestLookupEKSAction_ResolvesKnownRoutes(t *testing.T) { {"GET", "/clusters/alpha", "DescribeCluster", []string{"alpha"}}, {"DELETE", "/clusters/alpha", "DeleteCluster", []string{"alpha"}}, {"POST", "/clusters/alpha/update-config", "UpdateClusterConfig", []string{"alpha"}}, - {"POST", "/clusters/alpha/update-version", "UpdateClusterVersion", []string{"alpha"}}, + {"POST", "/clusters/alpha/updates", "UpdateClusterVersion", []string{"alpha"}}, + {"GET", "/clusters/alpha/updates", "ListUpdates", []string{"alpha"}}, + {"GET", "/clusters/alpha/updates/upd1", "DescribeUpdate", []string{"alpha", "upd1"}}, {"POST", "/clusters/alpha/node-groups", "CreateNodegroup", []string{"alpha"}}, {"GET", "/clusters/alpha/node-groups", "ListNodegroups", []string{"alpha"}}, {"GET", "/clusters/alpha/node-groups/ng1", "DescribeNodegroup", []string{"alpha", "ng1"}}, @@ -124,6 +126,8 @@ func TestLookupEKSAction_CoversAllActions(t *testing.T) { "ListClusters": false, "UpdateClusterConfig": false, "UpdateClusterVersion": false, + "ListUpdates": false, + "DescribeUpdate": false, "DeleteCluster": false, "CreateNodegroup": false, "DescribeNodegroup": false, @@ -226,3 +230,27 @@ type awsCodeError struct{ code string } func (e *awsCodeError) Error() string { return e.code } func errAWS(code string) error { return &awsCodeError{code: code} } + +// The update surface is absent, and the honest answer for an action that exists +// and is not served is NotImplemented. Unregistered, these two answered +// InvalidAction, which tells the caller they typed something that does not exist. +func TestLookupEKSAction_UpdateReadsRefuseAsNotImplemented(t *testing.T) { + for _, path := range []string{"/clusters/alpha/updates", "/clusters/alpha/updates/upd1"} { + t.Run(path, func(t *testing.T) { + _, params, handler, ok := eksRouter.lookup(http.MethodGet, path) + require.True(t, ok) + out, err := handler(context.Background(), nil, "000000000001", "", params, nil) + assert.Nil(t, out) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorNotImplemented, err.Error()) + }) + } +} + +// update-version is the nodegroup pattern. The cluster action has never been +// reachable at it on AWS, so keeping it registered would serve a path no caller +// sends and hide the one they do. +func TestLookupEKSAction_ClusterUpdateVersionIsNotAtTheNodegroupPath(t *testing.T) { + _, _, _, ok := eksRouter.lookup(http.MethodPost, "/clusters/alpha/update-version") + assert.False(t, ok) +} diff --git a/tests/integration/cross_tenant_test.go b/tests/integration/cross_tenant_test.go index eb19ccafd..1a3d98f40 100644 --- a/tests/integration/cross_tenant_test.go +++ b/tests/integration/cross_tenant_test.go @@ -428,11 +428,11 @@ func eksUncased() map[string]string { "AssociateAccessPolicy", "AssociateIdentityProviderConfig", "CreateAccessEntry", "CreateAddon", "CreateCluster", "CreateNodegroup", "DeleteAccessEntry", "DeleteAddon", "DeleteCluster", "DeleteNodegroup", "DescribeAccessEntry", "DescribeAddon", "DescribeAddonVersions", - "DescribeCluster", "DescribeIdentityProviderConfig", "DescribeNodegroup", + "DescribeCluster", "DescribeIdentityProviderConfig", "DescribeNodegroup", "DescribeUpdate", "DisassociateAccessPolicy", "DisassociateIdentityProviderConfig", "GetRecoveryDirective", "ListAccessEntries", "ListAccessPolicies", "ListAddons", "ListAssociatedAccessPolicies", "ListClusters", "ListIdentityProviderConfigs", "ListInternalAddons", "ListNodegroups", - "PublishInternal", "UpdateAccessEntry", "UpdateAddon", "UpdateClusterConfig", + "ListUpdates", "PublishInternal", "UpdateAccessEntry", "UpdateAddon", "UpdateClusterConfig", "UpdateClusterVersion", "UpdateNodegroupConfig", "UpdateNodegroupVersion", "WebhookTokenReview", } { uncased[action] = byName