Repository navigation
feat: configurable AWS Load Balancer Controller IRSA namespace/SA subjects - #62
Merged
Merged
Conversation
…jects Mirrors the external-secrets change: adds `aws_load_balancer_controller_namespace_service_accounts` (default keeps the current ["kube-system:aws-load-balancer-controller"], backward-compatible) and wires it into the ALB controller IRSA role's OIDC trust in place of the hardcoded subject. Threaded root -> comet_eks. Enables folding the AWS Load Balancer Controller into the comet-infra umbrella (comet-system ns): during migration the caller lists BOTH subjects ["kube-system:aws-load-balancer-controller", "comet-system:aws-load-balancer-controller"] for a zero-gap cutover, then trims to just comet-system. (main.tf shows alignment churn from terraform fmt widening the block; the only functional change is the one new pass-through line — verify with `git diff -w`.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
baz review on #62 (2 medium findings): 1. Validation: the *_namespace_service_accounts vars accepted arbitrary strings. The upstream module prepends "system:serviceaccount:" and matches with StringEquals, so a malformed or wildcard entry silently yields an IRSA trust the ServiceAccount can never assume. Add element validation ("<namespace>:<serviceaccount>" — exactly one colon, each part a DNS-1123 label, no wildcards) to BOTH the ALB and external-secrets vars at BOTH the root and child (comet_eks) boundaries. Tested: the regex accepts kube-system:aws-load-balancer-controller / comet-system:comet-infra-aws-load-balancer-controller / external-secrets:external-secrets and rejects wildcard/no-colon/extra-colon/empty/uppercase/underscore. 2. Stale docs: the ALB IRSA outputs (root + comet_eks) and the header comment hardcoded "annotate kube-system/aws-load-balancer-controller", which is wrong once the subject is configurable (and when folded into comet-infra the SA lives in comet-system). Reword to point at aws_load_balancer_controller_namespace_service_accounts as the source of truth. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
What
Adds
aws_load_balancer_controller_namespace_service_accounts(list of<ns>:<sa>OIDC subjects) to the ALB controller IRSA role, defaulting to the current["kube-system:aws-load-balancer-controller"](backward-compatible). Replaces the previously hardcoded subject inmodules/comet_eks/main.tf. Threaded root →comet_eks, mirroring the external-secrets change (#60).Why
Prerequisite for folding the AWS Load Balancer Controller into the
comet-infraumbrella (comet-system namespace). The IRSA trust is aStringEqualsexact-match on the SA subject, so moving the controller's SA breaks AWS auth unless the trust allows the new subject.During migration the caller lists both subjects for a zero-gap cutover. Note: the umbrella runs the controller with default (release-prefixed) naming — so the new SA is
comet-infra-aws-load-balancer-controller(chosen overfullnameOverrideto avoid a ClusterRole name-collision with the standalone app during the overlap). The dual-subject list is therefore:then a follow-up trims to just the umbrella subject.
Impact
Default unchanged → no diff for any existing cluster until a caller sets the new variable. Suggested release: v5.4.0.
Note:
main.tfshows alignment churn fromterraform fmt(the long variable name widened the=alignment of themodule.comet_eksblock). The only functional change is the single new pass-through line — confirm withgit diff -w.🤖 Generated with Claude Code
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Configure AWS Load Balancer Controller IRSA trust through
aws_load_balancer_controller_namespace_service_accounts, passing it from the root module intocomet_ekswhile preserving the existing default. Document the required ServiceAccount subject and validate namespace/name entries to support zero-gap migration to the comet-infra umbrella.Modified files (4)
Latest Contributors(2)
Modified files (2)
Latest Contributors(2)