diff --git a/lib/aws/eks_cluster_cr.go b/lib/aws/eks_cluster_cr.go index 3a42a01..00198f2 100644 --- a/lib/aws/eks_cluster_cr.go +++ b/lib/aws/eks_cluster_cr.go @@ -221,8 +221,13 @@ func (c *EKSCluster) irsaTrustPolicyForSA(subject string) pulumi.StringOutput { // WithAwsLbc installs the aws-load-balancer-controller: an IRSA role // (aws-load-balancer-controller SA in kube-system), the controller IAM policy + // attachment, the ServiceAccount, and the helm release. Mirrors with_aws_lbc. -// version is the controller image tag (config aws_lbc_version); chartVersion is -// the helm chart version (Python passes None → latest, so empty string here). +// version is the controller image tag override; chartVersion is the helm chart +// version. Both are optional: an empty version leaves image.tag unset so the +// chart supplies its own (correct) default image tag, and an empty chartVersion +// installs the latest chart. Python called with_aws_lbc() with no arguments, so +// both were None (image.tag omitted, latest chart); passing a non-empty value +// here would pin image.tag, which must be a real image tag (e.g. "v2.13.0"), +// NOT a chart version. func (c *EKSCluster) WithAwsLbc(version, chartVersion string) *EKSCluster { if c.err != nil { return c @@ -278,13 +283,18 @@ func (c *EKSCluster) WithAwsLbc(version, chartVersion string) *EKSCluster { values := pulumi.Map{ "clusterName": pulumi.String(c.cfg.Name), - "image": pulumi.Map{"tag": pulumi.String(version)}, "serviceAccount": pulumi.Map{ "create": pulumi.Bool(false), "name": pulumi.String("aws-load-balancer-controller"), }, "hostNetwork": pulumi.Bool(true), } + // Only pin image.tag when an explicit image tag is provided. Otherwise let the + // chart default the image; injecting an invalid tag (e.g. a chart version like + // "1.6.0") yields a non-existent image and ImagePullBackOff. + if version != "" { + values["image"] = pulumi.Map{"tag": pulumi.String(version)} + } relArgs := &helmv3.ReleaseArgs{ Chart: pulumi.String("aws-load-balancer-controller"), Namespace: pulumi.String("kube-system"), diff --git a/lib/aws/eks_cluster_cr_test.go b/lib/aws/eks_cluster_cr_test.go index 2abdb7e..71db6a5 100644 --- a/lib/aws/eks_cluster_cr_test.go +++ b/lib/aws/eks_cluster_cr_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" "github.com/pulumi/pulumi/sdk/v3/go/pulumi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -153,6 +154,44 @@ func TestCRAwsLbcPolicyAndAttachment(t *testing.T) { "aws-lbc attachment must NOT alias under the role") } +// TestCRAwsLbcImageTag asserts the aws-lbc helm Release only pins +// values.image.tag when an explicit image tag is supplied, and omits it +// otherwise. Regression guard for the ImagePullBackOff bug where the +// aws_lbc_version config default (a chart version like "1.6.0") was injected as +// the image tag, producing a non-existent +// public.ecr.aws/eks/aws-load-balancer-controller:1.6.0 image. +func TestCRAwsLbcImageTag(t *testing.T) { + imageTagOf := func(version string) (resource.PropertyValue, bool) { + mocks := &eksMocks{} + err := pulumi.RunErr(func(ctx *pulumi.Context) error { + c, err := NewEKSCluster(ctx, crBaseCfg()) + require.NoError(t, err) + c.WithNodeRole("").WithAwsLbc(version, "") + return c.Err() + }, pulumi.WithMocks(crProject, crName, mocks)) + require.NoError(t, err) + + releases := mocks.byType("kubernetes:helm.sh/v3:Release") + require.Len(t, releases, 1, "expected exactly one aws-lbc helm Release") + values := releases[0].Inputs["values"].ObjectValue() + image, hasImage := values["image"] + if !hasImage { + return resource.PropertyValue{}, false + } + tag, hasTag := image.ObjectValue()["tag"] + return tag, hasTag + } + + // Empty version → no image.tag override (chart supplies its default image). + _, hasTag := imageTagOf("") + assert.False(t, hasTag, "empty version must NOT set values.image.tag") + + // Explicit image tag → pinned verbatim. + tag, hasTag := imageTagOf("v2.13.0") + require.True(t, hasTag, "explicit version must set values.image.tag") + assert.Equal(t, "v2.13.0", tag.StringValue()) +} + // TestCRMimirPolicyAttachmentAndReleaseParent asserts the mimir storage Policy + // attachment use the Policy-child chain, and the mimir helm Release aliases under // the mimir Namespace, matching the live state: diff --git a/lib/steps/cluster.go b/lib/steps/cluster.go index d341fa0..02c6eb9 100644 --- a/lib/steps/cluster.go +++ b/lib/steps/cluster.go @@ -225,11 +225,17 @@ func (s *ClusterStep) runAWSInlineGo(ctx context.Context) error { } params := awsClusterParams{ - compoundName: compoundName, - clusterName: clusterName, - region: region, - accountID: awsCreds.AccountID(), - iamPermissionsBoundaryARN: fmt.Sprintf("arn:aws:iam::%s:policy/PositTeamDedicatedAdmin", awsCreds.AccountID()), + compoundName: compoundName, + clusterName: clusterName, + region: region, + accountID: awsCreds.AccountID(), + // Python's control-room _define_eks does NOT set a permissions_boundary on the + // EKS cluster / node / IRSA roles (unlike the workload paths, which do), so the + // live control-room roles carry no boundary. The admin identity used for + // control-room applies also cannot set one (iam:PutRolePermissionsBoundary is + // denied). Pass empty so the builder omits the boundary — the builder guards + // each role with `if IAMPermissionsBoundary != ""`, so this is a no-op diff. + iamPermissionsBoundaryARN: "", requiredTags: buildClusterRequiredTags(cfg), cfg: cfg, credentials: awsCreds, diff --git a/lib/steps/cluster_aws.go b/lib/steps/cluster_aws.go index 2d00124..28c0d31 100644 --- a/lib/steps/cluster_aws.go +++ b/lib/steps/cluster_aws.go @@ -299,7 +299,11 @@ func awsClusterDeploy(ctx *pulumi.Context, _ types.Target, params awsClusterPara // workload path passes the .posit.team-suffixed name explicitly; do NOT // conflate the two. State URN: …$aws:iam/role:Role::-ebs-csi-driver. WithEbsCsiDriver(name+"-ebs-csi-driver", cfg.EBSCsiAddonVersion()). - WithAwsLbc(cfg.AWSLbcVersion(), ""). + // No image-tag or chart-version override: image.tag is omitted so the chart + // supplies its own default image, and the chart version defaults to latest. + // Do NOT pass a chart version (e.g. "1.6.0") as the image tag here; that + // yields a non-existent image and ImagePullBackOff. + WithAwsLbc("", ""). WithMetricsServer(cfg.MetricsServerVersionOrDefault()). WithSecretStoreCsi(cfg.SecretStoreCsiVersionOrDefault()). WithSecretStoreCsiAwsProvider(cfg.SecretStoreCsiAwsProviderVersionOrDefault()) diff --git a/lib/steps/cluster_aws_test.go b/lib/steps/cluster_aws_test.go index 96bd9f1..76e6382 100644 --- a/lib/steps/cluster_aws_test.go +++ b/lib/steps/cluster_aws_test.go @@ -122,11 +122,13 @@ func newTestClusterParams() awsClusterParams { ResourceTags: map[string]string{}, } return awsClusterParams{ - compoundName: cn, - clusterName: cn, // control-room cluster logical name = compound name - region: "us-east-2", - accountID: "123456789012", - iamPermissionsBoundaryARN: "arn:aws:iam::123456789012:policy/PositTeamDedicatedAdmin", + compoundName: cn, + clusterName: cn, // control-room cluster logical name = compound name + region: "us-east-2", + accountID: "123456789012", + // Control-room roles carry NO permissions boundary (matches Python + live + // state); the step passes empty. See TestAWSClusterDeployNoPermissionsBoundary. + iamPermissionsBoundaryARN: "", requiredTags: buildClusterRequiredTags(cfg), cfg: cfg, subnetIDs: []string{"subnet-a", "subnet-b"}, @@ -167,6 +169,26 @@ func TestAWSClusterDeployControlPlaneLogicalName(t *testing.T) { assert.Equal(t, resource.NewStringProperty("cr01-staging"), clusters[0].Inputs["name"]) } +func TestAWSClusterDeployNoPermissionsBoundary(t *testing.T) { + mocks := runClusterDeploy(t, newTestClusterParams()) + + // Control-room IAM roles must NOT carry a permissions boundary. Python's + // control-room _define_eks does not set one, the live control-room roles have + // none, and the control-room admin identity cannot call + // iam:PutRolePermissionsBoundary. Every role the cluster deploy creates should + // therefore have an unset/empty permissionsBoundary input. + roles := mocks.byType("aws:iam/role:Role") + require.NotEmpty(t, roles) + for _, r := range roles { + pb, ok := r.Inputs["permissionsBoundary"] + if !ok || pb.IsNull() { + continue + } + assert.Equalf(t, "", pb.StringValue(), + "control-room role %q must have no permissions boundary", r.Name) + } +} + func TestAWSClusterDeployACMAndValidation(t *testing.T) { mocks := runClusterDeploy(t, newTestClusterParams()) diff --git a/lib/types/controlroom.go b/lib/types/controlroom.go index db56d45..d5aaa31 100644 --- a/lib/types/controlroom.go +++ b/lib/types/controlroom.go @@ -66,7 +66,6 @@ type AWSControlRoomConfig struct { TrustedUsers []TrustedUser `json:"trusted_users" yaml:"trusted_users"` FrontDoor *string `json:"front_door" yaml:"front_door"` AwsFsxOpenzfsCsiVersion string `json:"aws_fsx_openzfs_csi_version" yaml:"aws_fsx_openzfs_csi_version"` - AwsLbcVersion string `json:"aws_lbc_version" yaml:"aws_lbc_version"` ExternalDnsVersion string `json:"external_dns_version" yaml:"external_dns_version"` GrafanaVersion string `json:"grafana_version" yaml:"grafana_version"` KubeStateMetricsVersion string `json:"kube_state_metrics_version" yaml:"kube_state_metrics_version"` @@ -139,11 +138,6 @@ func (c AWSControlRoomConfig) EBSCsiAddonVersion() string { return crStringDefault(c.EbsCsiAddonVersion, "v1.41.0-eksbuild.1") } -// AWSLbcVersion resolves the aws-load-balancer-controller version (Python default 1.6.0). -func (c AWSControlRoomConfig) AWSLbcVersion() string { - return crStringDefault(c.AwsLbcVersion, "1.6.0") -} - // MetricsServerVersion resolves the metrics-server chart version (Python default 3.11.0). func (c AWSControlRoomConfig) MetricsServerVersionOrDefault() string { return crStringDefault(c.MetricsServerVersion, "3.11.0")