POC: Replace maestro with kube-applier and hyperfleet stack with hyperfleet-operator using KAS as db#641
Open
psav wants to merge 37 commits into
Open
POC: Replace maestro with kube-applier and hyperfleet stack with hyperfleet-operator using KAS as db#641psav wants to merge 37 commits into
psav wants to merge 37 commits into
Conversation
Introduces kube-applier-aws as a per-MC controller that reads desires from DynamoDB (in the RC account) and applies them to the local cluster. Changes: - Helm chart: argocd/config/management-cluster/kube-applier/ deploys the quay.io/psav/kube-applier-aws:latest image with correct args derived from global.cluster_name and global.aws_region injected by ArgoCD. Includes RBAC, ServiceAccount, Service, and optional ServiceMonitor. - terraform/modules/kube-applier: EKS Pod Identity role for the MC pod with scoped DynamoDB permissions (specs: read+streams, status: rw) targeting tables in the RC account. - terraform/modules/kube-applier-dynamodb: creates 6 DynamoDB tables per MC (3 specs with streams, 3 status) in the RC account, plus a backend IAM role for the future desire-writer service. - terraform/config/kube-applier-dynamodb-provisioning: top-level Terraform config called per-MC in the RC account (analogous to maestro-agent-iot-provisioning). - scripts/buildspec/dynamodb-mint.sh + buildspec-dynamodb-mint.yml: mint step that runs in RC account credentials to provision DynamoDB tables, parallel with the existing Mint-IoT action. - terraform/config/pipeline-management-cluster/main.tf: adds the MintDynamoDB CodeBuild project and a parallel action in the Mint-IoT stage so both mints run concurrently.
feat: add kube-applier-aws helm chart, IAM, and DynamoDB provisioning
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the kube-applier Helm chart, Terraform modules for controller IAM and DynamoDB tables, management-cluster and pipeline wiring, and a CodeBuild-based mint flow for provisioning the backing tables. Changeskube-applier rollout and DynamoDB minting
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
terraform/config/pipeline-management-cluster/buildspec-dynamodb-mint.yml (1)
12-18: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the unused output artifact or narrow it.
dynamodb_mint_outputis not consumed by any later stage in this pipeline, so packaging**/*just uploads the repo after every run. Either remove the action output artifact or restrict this artifact set to files a downstream stage actually needs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/config/pipeline-management-cluster/buildspec-dynamodb-mint.yml` around lines 12 - 18, The artifact configuration in the buildspec is too broad for the unused dynamodb_mint_output output and is packaging the entire repository. Update the artifacts section to either remove this output artifact entirely or narrow the files list to only what a downstream stage actually consumes, keeping the existing exclude-paths behavior if applicable.argocd/config/management-cluster/kube-applier/templates/deployment.yaml (1)
25-27: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden the container filesystem as well.
The pod already opts into
runAsNonRootandRuntimeDefault, but the container still has a writable root filesystem. If the image only writes to mounted volumes, add a container-levelsecurityContextwithreadOnlyRootFilesystem: true(and typicallyallowPrivilegeEscalation: false) to reduce post-compromise blast radius. Please verify the image behavior first.Suggested change
containers: - name: kube-applier image: "{{ .Values.kubeApplier.image.registry }}/{{ .Values.kubeApplier.image.repository }}:{{ .Values.kubeApplier.image.tag }}" imagePullPolicy: {{ .Values.kubeApplier.image.pullPolicy }} + securityContext: + readOnlyRootFilesystem: true + allowPrivilegeEscalation: false args:Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml` around lines 25 - 27, The kube-applier container is already running as non-root with RuntimeDefault, but its container-level hardening is incomplete because the root filesystem remains writable. Update the kube-applier container spec in the deployment template to add a container securityContext with readOnlyRootFilesystem enabled and allowPrivilegeEscalation disabled, and verify the image only writes to mounted volumes before applying the change.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@argocd/config/management-cluster/kube-applier/templates/_helpers.tpl`:
- Around line 49-51: The `kube-applier.serviceAccountName` helper is currently
pulling from `Values.kubeApplier.serviceAccount.name`, which allows drift from
the fixed Pod Identity binding. Update this helper to always return the required
`kube-applier` ServiceAccount name, and remove or ignore the override path so
the Helm chart stays consistent with the IAM Pod Identity configuration.
In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml`:
- Around line 34-35: The kube-applier deployment template currently lets metrics
and health listen addresses be configured via the container args, but the
containerPort entries and readiness/liveness probe targets are still hardcoded
to the default ports. Update the deployment template so the args, named
container ports, and probe endpoints all read from the same configurable values
in .Values.kubeApplier.config (for example the metricsListenAddress and
healthzListenAddress settings) to keep them in sync when non-default ports are
used.
In `@argocd/config/management-cluster/kube-applier/templates/service.yaml`:
- Line 6: The service template is using .Release.Namespace, which can drift from
the fixed namespace expected by the Pod Identity association. Update the
namespace handling in the kube-applier service manifest to consistently target
the kube-applier namespace, and align it with the same namespace used by the
IAM/Pod Identity binding and related management-cluster resources. Use the
service template’s namespace field and any associated Helm values/helpers to
enforce this namespace instead of relying on the release namespace.
In `@argocd/config/management-cluster/kube-applier/values.yaml`:
- Around line 10-11: The kube-applier image is using a mutable latest tag, which
makes deployments non-deterministic. Update the image configuration in
values.yaml to use a pinned version or digest instead of tag: "latest", and keep
the existing pullPolicy consistent with the new immutable reference. Use the
image tag settings under kube-applier to locate and replace this value.
In `@scripts/buildspec/dynamodb-mint.sh`:
- Around line 15-23: The REGIONAL_ID lookup in dynamodb-mint.sh can return the
literal string null when jq reads a missing regional_id, so validate the value
immediately after the jq call in the REGIONAL_ID assignment block and exit with
an error if it is empty or null. Update the logic around RC_CONFIG_FILE and the
REGIONAL_ID fetch so the script fails before writing tfvars, rather than letting
Terraform hit the bad value later.
In `@terraform/config/kube-applier-dynamodb-provisioning/main.tf`:
- Around line 1-21: The AWS region is currently implicit because the provider
block in main.tf does not set provider.aws.region and the module input uses
data.aws_region.current.name, so the stack may deploy into the caller’s default
region. Update the aws provider to take region from a new aws_region variable,
remove the aws_region lookup via data.aws_region.current, and pass that same
variable into module.kube_applier_dynamodb so the module and provider use a
consistent explicit region.
In `@terraform/config/pipeline-management-cluster/main.tf`:
- Around line 489-527: The new aws_codebuild_project.dynamodb_mint is using the
default CodeBuild encryption behavior instead of the pipeline customer-managed
KMS key. Update the CodeBuild project configuration to explicitly use the
pipeline CMK by wiring in the existing KMS key reference used by the pipeline,
so the encryption settings for dynamodb_mint are pinned to the same managed key
as the rest of the pipeline resources.
- Around line 489-527: The same-account fallback in
aws_iam_role_policy.codebuild_policy is missing DynamoDB permissions, so
DynamoDB Mint fails when scripts/buildspec/dynamodb-mint.sh does not hop roles.
Update the same-account allowlist to include the DynamoDB table actions needed
by the Terraform flow for aws_codebuild_project.dynamodb_mint, alongside the
existing IoT/EKS/KMS/IAM permissions, so same-account runs can create the tables
successfully.
In `@terraform/modules/kube-applier-dynamodb/iam.tf`:
- Around line 17-49: The kube_applier_backend IAM role is being created as a
regional singleton from a per-MC module, so multiple module instances will
collide on the same role name. Update the aws_iam_role "kube_applier_backend"
path to either move this shared role into a separate regional module/state or
add an explicit create-vs-reference switch so per-MC instances do not all try to
create it. Keep the role identity tied to the shared regional scope, and do not
rely on the empty lifecycle ignore_changes block to avoid the duplicate
creation.
- Around line 21-33: The Pod Identity trust policy in the IAM role is too broad
and should be scoped to the intended RC cluster. Update the assume_role_policy
for the role defined in iam.tf to add aws:SourceAccount and aws:SourceArn
conditions alongside the existing pods.eks.amazonaws.com principal, so only the
expected cluster can assume this role. Keep the change localized to the assume
role policy JSON in the kube-applier-dynamodb IAM configuration.
In `@terraform/modules/kube-applier/versions.tf`:
- Around line 4-8: The AWS provider constraint in the required_providers block
is too low for aws_eks_pod_identity_association and can resolve to incompatible
releases. Update the version floor in the versions.tf required_providers entry
for hashicorp/aws to at least 5.29.0, keeping the change scoped to the aws
provider constraint so terraform init/plan always selects a compatible provider.
---
Nitpick comments:
In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml`:
- Around line 25-27: The kube-applier container is already running as non-root
with RuntimeDefault, but its container-level hardening is incomplete because the
root filesystem remains writable. Update the kube-applier container spec in the
deployment template to add a container securityContext with
readOnlyRootFilesystem enabled and allowPrivilegeEscalation disabled, and verify
the image only writes to mounted volumes before applying the change.
In `@terraform/config/pipeline-management-cluster/buildspec-dynamodb-mint.yml`:
- Around line 12-18: The artifact configuration in the buildspec is too broad
for the unused dynamodb_mint_output output and is packaging the entire
repository. Update the artifacts section to either remove this output artifact
entirely or narrow the files list to only what a downstream stage actually
consumes, keeping the existing exclude-paths behavior if applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 885f1665-fa74-424f-9416-58329915e4ac
📒 Files selected for processing (32)
argocd/config/management-cluster/kube-applier/Chart.yamlargocd/config/management-cluster/kube-applier/templates/_helpers.tplargocd/config/management-cluster/kube-applier/templates/clusterrole.yamlargocd/config/management-cluster/kube-applier/templates/clusterrolebinding.yamlargocd/config/management-cluster/kube-applier/templates/deployment.yamlargocd/config/management-cluster/kube-applier/templates/namespace.yamlargocd/config/management-cluster/kube-applier/templates/service.yamlargocd/config/management-cluster/kube-applier/templates/serviceaccount.yamlargocd/config/management-cluster/kube-applier/templates/servicemonitor.yamlargocd/config/management-cluster/kube-applier/values.yamlscripts/buildspec/dynamodb-mint.shterraform/config/kube-applier-dynamodb-provisioning/backend.tfterraform/config/kube-applier-dynamodb-provisioning/main.tfterraform/config/kube-applier-dynamodb-provisioning/outputs.tfterraform/config/kube-applier-dynamodb-provisioning/terraform.tfvars.exampleterraform/config/kube-applier-dynamodb-provisioning/variables.tfterraform/config/management-cluster/main.tfterraform/config/management-cluster/outputs.tfterraform/config/pipeline-management-cluster/buildspec-dynamodb-mint.ymlterraform/config/pipeline-management-cluster/main.tfterraform/modules/kube-applier-dynamodb/README.mdterraform/modules/kube-applier-dynamodb/iam.tfterraform/modules/kube-applier-dynamodb/main.tfterraform/modules/kube-applier-dynamodb/outputs.tfterraform/modules/kube-applier-dynamodb/variables.tfterraform/modules/kube-applier-dynamodb/versions.tfterraform/modules/kube-applier/README.mdterraform/modules/kube-applier/iam.tfterraform/modules/kube-applier/main.tfterraform/modules/kube-applier/outputs.tfterraform/modules/kube-applier/variables.tfterraform/modules/kube-applier/versions.tf
Comment on lines
+49
to
+51
| {{- define "kube-applier.serviceAccountName" -}} | ||
| {{ .Values.kubeApplier.serviceAccount.name }} | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce the fixed ServiceAccount name required by Pod Identity.
This helper allows overriding the ServiceAccount name, but Terraform binds IAM Pod Identity specifically to service_account = "kube-applier". A drift here will break AWS credential access at runtime.
Suggested fix
{{- define "kube-applier.serviceAccountName" -}}
-{{ .Values.kubeApplier.serviceAccount.name }}
+{{- $sa := default "kube-applier" .Values.kubeApplier.serviceAccount.name -}}
+{{- if ne $sa "kube-applier" -}}
+{{- fail "kubeApplier.serviceAccount.name must be 'kube-applier' to match aws_eks_pod_identity_association" -}}
+{{- end -}}
+{{- $sa -}}
{{- end }}As per path instructions, "Review Helm values for management cluster services ... Ensure proper namespace scoping and consistency with MC architecture patterns."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- define "kube-applier.serviceAccountName" -}} | |
| {{ .Values.kubeApplier.serviceAccount.name }} | |
| {{- end }} | |
| {{- define "kube-applier.serviceAccountName" -}} | |
| {{- $sa := default "kube-applier" .Values.kubeApplier.serviceAccount.name -}} | |
| {{- if ne $sa "kube-applier" -}} | |
| {{- fail "kubeApplier.serviceAccount.name must be 'kube-applier' to match aws_eks_pod_identity_association" -}} | |
| {{- end -}} | |
| {{- $sa -}} | |
| {{- end }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@argocd/config/management-cluster/kube-applier/templates/_helpers.tpl` around
lines 49 - 51, The `kube-applier.serviceAccountName` helper is currently pulling
from `Values.kubeApplier.serviceAccount.name`, which allows drift from the fixed
Pod Identity binding. Update this helper to always return the required
`kube-applier` ServiceAccount name, and remove or ignore the override path so
the Helm chart stays consistent with the IAM Pod Identity configuration.
Source: Path instructions
| kind: Service | ||
| metadata: | ||
| name: {{ include "kube-applier.fullname" . }} | ||
| namespace: {{ .Release.Namespace }} |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce kube-applier namespace to match Pod Identity binding.
This template trusts .Release.Namespace, but the IAM association is created for namespace kube-applier. If release namespace drifts, controller auth to DynamoDB fails.
Suggested fix
+{{- if ne .Release.Namespace "kube-applier" -}}
+{{- fail "Release.Namespace must be 'kube-applier' to match aws_eks_pod_identity_association" -}}
+{{- end }}
apiVersion: v1
kind: ServiceAs per path instructions, "Review Helm values for management cluster services ... Ensure proper namespace scoping and consistency with MC architecture patterns."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@argocd/config/management-cluster/kube-applier/templates/service.yaml` at line
6, The service template is using .Release.Namespace, which can drift from the
fixed namespace expected by the Pod Identity association. Update the namespace
handling in the kube-applier service manifest to consistently target the
kube-applier namespace, and align it with the same namespace used by the IAM/Pod
Identity binding and related management-cluster resources. Use the service
template’s namespace field and any associated Helm values/helpers to enforce
this namespace instead of relying on the release namespace.
Source: Path instructions
Comment on lines
+15
to
+23
| if [ -z "${REGIONAL_ID:-}" ]; then | ||
| RC_CONFIG_FILE="deploy/${ENVIRONMENT}/${TARGET_REGION}/pipeline-regional-cluster-inputs/terraform.json" | ||
| if [ -f "$RC_CONFIG_FILE" ]; then | ||
| REGIONAL_ID=$(jq -r '.regional_id' "$RC_CONFIG_FILE") | ||
| else | ||
| echo "ERROR: Cannot determine REGIONAL_ID — not set and RC config not found: $RC_CONFIG_FILE" >&2 | ||
| exit 1 | ||
| fi | ||
| fi |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject a missing regional_id immediately.
jq -r '.regional_id' returns the literal string null when the key is absent, so this branch can silently write regional_id = "null" into the tfvars file and fail much later inside Terraform. Validate the result right after Line 18 and stop here instead.
Suggested fix
if [ -z "${REGIONAL_ID:-}" ]; then
RC_CONFIG_FILE="deploy/${ENVIRONMENT}/${TARGET_REGION}/pipeline-regional-cluster-inputs/terraform.json"
if [ -f "$RC_CONFIG_FILE" ]; then
REGIONAL_ID=$(jq -r '.regional_id' "$RC_CONFIG_FILE")
+ if [ -z "$REGIONAL_ID" ] || [ "$REGIONAL_ID" = "null" ]; then
+ echo "ERROR: regional_id is missing in $RC_CONFIG_FILE" >&2
+ exit 1
+ fi
else
echo "ERROR: Cannot determine REGIONAL_ID — not set and RC config not found: $RC_CONFIG_FILE" >&2
exit 1
fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ -z "${REGIONAL_ID:-}" ]; then | |
| RC_CONFIG_FILE="deploy/${ENVIRONMENT}/${TARGET_REGION}/pipeline-regional-cluster-inputs/terraform.json" | |
| if [ -f "$RC_CONFIG_FILE" ]; then | |
| REGIONAL_ID=$(jq -r '.regional_id' "$RC_CONFIG_FILE") | |
| else | |
| echo "ERROR: Cannot determine REGIONAL_ID — not set and RC config not found: $RC_CONFIG_FILE" >&2 | |
| exit 1 | |
| fi | |
| fi | |
| if [ -z "${REGIONAL_ID:-}" ]; then | |
| RC_CONFIG_FILE="deploy/${ENVIRONMENT}/${TARGET_REGION}/pipeline-regional-cluster-inputs/terraform.json" | |
| if [ -f "$RC_CONFIG_FILE" ]; then | |
| REGIONAL_ID=$(jq -r '.regional_id' "$RC_CONFIG_FILE") | |
| if [ -z "$REGIONAL_ID" ] || [ "$REGIONAL_ID" = "null" ]; then | |
| echo "ERROR: regional_id is missing in $RC_CONFIG_FILE" >&2 | |
| exit 1 | |
| fi | |
| else | |
| echo "ERROR: Cannot determine REGIONAL_ID — not set and RC config not found: $RC_CONFIG_FILE" >&2 | |
| exit 1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/buildspec/dynamodb-mint.sh` around lines 15 - 23, The REGIONAL_ID
lookup in dynamodb-mint.sh can return the literal string null when jq reads a
missing regional_id, so validate the value immediately after the jq call in the
REGIONAL_ID assignment block and exit with an error if it is empty or null.
Update the logic around RC_CONFIG_FILE and the REGIONAL_ID fetch so the script
fails before writing tfvars, rather than letting Terraform hit the bad value
later.
Comment on lines
+489
to
+527
| # CodeBuild Project - DynamoDB Mint (runs in RC account context, parallel with Mint-IoT) | ||
| resource "aws_codebuild_project" "dynamodb_mint" { | ||
| name = local.dynamodb_mint_project_name | ||
| service_role = aws_iam_role.codebuild_role.arn | ||
| build_timeout = 30 | ||
|
|
||
| artifacts { | ||
| type = "CODEPIPELINE" | ||
| } | ||
|
|
||
| environment { | ||
| compute_type = "BUILD_GENERAL1_SMALL" | ||
| image = var.codebuild_image | ||
| type = "LINUX_CONTAINER" | ||
| image_pull_credentials_type = "CODEBUILD" | ||
|
|
||
| environment_variable { | ||
| name = "TARGET_ACCOUNT_ID" | ||
| value = var.target_account_id | ||
| } | ||
| environment_variable { | ||
| name = "TARGET_REGION" | ||
| value = var.target_region | ||
| } | ||
| environment_variable { | ||
| name = "MANAGEMENT_ID" | ||
| value = var.management_id | ||
| } | ||
| environment_variable { | ||
| name = "ENVIRONMENT" | ||
| value = var.target_environment | ||
| } | ||
| } | ||
|
|
||
| source { | ||
| type = "CODEPIPELINE" | ||
| buildspec = "terraform/config/pipeline-management-cluster/buildspec-dynamodb-mint.yml" | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use a customer-managed KMS key on the new CodeBuild project.
This project currently relies on the default encryption behavior. Since it handles generated Terraform inputs and cross-account infra changes, it should be pinned to the pipeline CMK instead of AWS-managed encryption.
🧰 Tools
🪛 Checkov (3.3.1)
[medium] 490-527: Ensure that CodeBuild projects are encrypted using CMK
(CKV_AWS_147)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/config/pipeline-management-cluster/main.tf` around lines 489 - 527,
The new aws_codebuild_project.dynamodb_mint is using the default CodeBuild
encryption behavior instead of the pipeline customer-managed KMS key. Update the
CodeBuild project configuration to explicitly use the pipeline CMK by wiring in
the existing KMS key reference used by the pipeline, so the encryption settings
for dynamodb_mint are pinned to the same managed key as the rest of the pipeline
resources.
Source: Linters/SAST tools
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add same-account DynamoDB permissions for this build.
scripts/buildspec/dynamodb-mint.sh always switches into the RC account, but when the RC account is the same as the current account there is no role hop and this project falls back to the same-account allowlist in aws_iam_role_policy.codebuild_policy. That block grants IoT/EKS/KMS/IAM permissions, but no DynamoDB table actions, so same-account runs will fail with AccessDenied as soon as Terraform tries to create the tables.
Suggested fix
{
Effect = "Allow"
Action = [
+ # DynamoDB - For kube-applier table provisioning in same-account runs
+ "dynamodb:CreateTable",
+ "dynamodb:DeleteTable",
+ "dynamodb:DescribeTable",
+ "dynamodb:DescribeContinuousBackups",
+ "dynamodb:UpdateContinuousBackups",
+ "dynamodb:TagResource",
+ "dynamodb:UntagResource",
+ "dynamodb:UpdateTable",
# IoT - For minting Maestro agent certificates (same-account case)
"iot:*",🧰 Tools
🪛 Checkov (3.3.1)
[medium] 490-527: Ensure that CodeBuild projects are encrypted using CMK
(CKV_AWS_147)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/config/pipeline-management-cluster/main.tf` around lines 489 - 527,
The same-account fallback in aws_iam_role_policy.codebuild_policy is missing
DynamoDB permissions, so DynamoDB Mint fails when
scripts/buildspec/dynamodb-mint.sh does not hop roles. Update the same-account
allowlist to include the DynamoDB table actions needed by the Terraform flow for
aws_codebuild_project.dynamodb_mint, alongside the existing IoT/EKS/KMS/IAM
permissions, so same-account runs can create the tables successfully.
Comment on lines
+4
to
+8
| required_providers { | ||
| aws = { | ||
| source = "hashicorp/aws" | ||
| version = ">= 5.0" | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
What is the first version of the Terraform hashicorp/aws provider that supports the aws_eks_pod_identity_association resource?
💡 Result:
The first version of the Terraform hashicorp/aws provider that supports the aws_eks_pod_identity_association resource is v5.29.0, which was released on December 1, 2023 [1].
Citations:
Raise the AWS provider floor to 5.29.0. aws_eks_pod_identity_association needs hashicorp/aws v5.29.0+, so version = ">= 5.0" can resolve to releases that fail during terraform init/plan unless the caller’s lockfile pins a newer provider.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/modules/kube-applier/versions.tf` around lines 4 - 8, The AWS
provider constraint in the required_providers block is too low for
aws_eks_pod_identity_association and can resolve to incompatible releases.
Update the version floor in the versions.tf required_providers entry for
hashicorp/aws to at least 5.29.0, keeping the change scoped to the aws provider
constraint so terraform init/plan always selects a compatible provider.
- deployment.yaml: derive containerPort numbers from metricsListenAddress / healthzListenAddress values via trimPrefix to prevent drift if ports change - pipeline-management-cluster/main.tf: add dynamodb:* to same-account IAM policy so the dynamodb-mint CodeBuild job can provision tables in ephemeral environments (same-account case) — analogous to iot:* for iot-mint - kube-applier-dynamodb/iam.tf: remove backend IAM role; the module is called once per MC with separate state, so a shared RC-scoped role with the same name would fail with EntityAlreadyExists on the second MC. The role will be added to the RC-level module when the backend service is built - kube-applier-dynamodb/outputs.tf, kube-applier-dynamodb-provisioning/outputs.tf: remove backend_role_arn outputs that referenced the removed role
fix: action valid CodeRabbit comments on kube-applier-aws
kube-applier runs in the MC account but the DynamoDB tables live in the RC account. When the SDK receives a bare table name it resolves it against the caller's own account, so the pod was hitting the MC account (where no tables exist) instead of the RC account. Solution: pass full cross-account ARNs as the --specs-table / --status-table flags. The DynamoDB SDK accepts ARNs in the TableName field, so no application code change is needed. Changes: - deployment.yaml: build ARN from global.aws_region + global.rc_aws_account_id instead of bare name prefix - values.yaml: add global.rc_aws_account_id placeholder - applicationset.yaml.j2: inject rc_aws_account_id from cluster Secret annotation - ecs-bootstrap/main.tf: write rc_aws_account_id into cluster Secret annotation and pass it as RC_AWS_ACCOUNT_ID env var to the bootstrap container - ecs-bootstrap/variables.tf: add rc_aws_account_id variable - management-cluster/main.tf: pass regional_aws_account_id -> rc_aws_account_id into ecs_bootstrap module - Re-rendered deploy/ applicationset files for ephemeral + integration
…ynamodb fix: pass DynamoDB table ARNs for cross-account access from MC pod
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
argocd/config/management-cluster/kube-applier/templates/deployment.yaml (1)
25-75: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHarden container filesystem to read-only.
The container is missing
securityContext.readOnlyRootFilesystem: true, which leaves writable root FS and weakens runtime hardening.Suggested patch
containers: - name: kube-applier + securityContext: + readOnlyRootFilesystem: true image: "{{ .Values.kubeApplier.image.registry }}/{{ .Values.kubeApplier.image.repository }}:{{ .Values.kubeApplier.image.tag }}" imagePullPolicy: {{ .Values.kubeApplier.image.pullPolicy }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml` around lines 25 - 75, The kube-applier container spec is missing the read-only root filesystem hardening setting. Update the container-level security settings in the deployment template for the kube-applier workload and add readOnlyRootFilesystem: true alongside the existing securityContext fields so the container runs with a non-writable root filesystem.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@terraform/modules/ecs-bootstrap/variables.tf`:
- Around line 68-72: Add module-boundary validation for rc_aws_account_id in the
variable block so malformed account IDs are rejected early; keep empty string
allowed if that is the intended default, otherwise enforce a 12-digit numeric
AWS account format with a validation block on variable "rc_aws_account_id". Use
the variable declaration itself as the fix location and ensure the condition
matches the existing empty default plus valid account ID constraint.
---
Outside diff comments:
In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml`:
- Around line 25-75: The kube-applier container spec is missing the read-only
root filesystem hardening setting. Update the container-level security settings
in the deployment template for the kube-applier workload and add
readOnlyRootFilesystem: true alongside the existing securityContext fields so
the container runs with a non-writable root filesystem.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b6207ee8-63ae-4d61-9f59-c5f27ee72a46
📒 Files selected for processing (10)
argocd/config/management-cluster/kube-applier/templates/deployment.yamlargocd/config/management-cluster/kube-applier/values.yamlconfig/templates/argocd-bootstrap/applicationset.yaml.j2deploy/ephemeral/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/ephemeral/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamlterraform/config/management-cluster/main.tfterraform/modules/ecs-bootstrap/main.tfterraform/modules/ecs-bootstrap/variables.tf
✅ Files skipped from review due to trivial changes (3)
- deploy/integration/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yaml
- deploy/ephemeral/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yaml
- argocd/config/management-cluster/kube-applier/values.yaml
Comment on lines
+68
to
+72
| variable "rc_aws_account_id" { | ||
| description = "AWS account ID of the regional cluster where DynamoDB tables reside. Injected into the ArgoCD cluster secret so charts can build cross-account resource ARNs." | ||
| type = string | ||
| default = "" | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add input validation for rc_aws_account_id at the module boundary.
This variable currently accepts arbitrary strings; malformed values will flow into cross-account ARN construction and fail at runtime. Add a validation block (e.g., allow empty string or enforce ^[0-9]{12}$).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/modules/ecs-bootstrap/variables.tf` around lines 68 - 72, Add
module-boundary validation for rc_aws_account_id in the variable block so
malformed account IDs are rejected early; keep empty string allowed if that is
the intended default, otherwise enforce a 12-digit numeric AWS account format
with a validation block on variable "rc_aws_account_id". Use the variable
declaration itself as the fix location and ensure the condition matches the
existing empty default plus valid account ID constraint.
DynamoDB cross-account access requires both an identity-based policy on the caller's role AND a resource-based policy on the table itself. The identity policy was already in place (kube-applier/iam.tf); the resource policies were missing, causing AccessDeniedException with 'no resource-based policy allows'. Add aws_dynamodb_resource_policy resources to each of the six tables: - Specs tables: GetItem/Scan/Query + DescribeStream/GetRecords/GetShardIterator/ListStreams - Status tables: GetItem/Scan/Query/PutItem/DeleteItem The MC kube-applier role ARN is constructed from mc_aws_account_id + mc_name, which follows the naming convention set in terraform/modules/kube-applier/iam.tf. Thread mc_aws_account_id (= TARGET_ACCOUNT_ID in the pipeline) through: dynamodb-mint.sh -> kube-applier-dynamodb-provisioning -> kube-applier-dynamodb module
…ce-policy fix: add DynamoDB resource-based policies for cross-account MC access
DynamoDB resource-based policies do not support Streams API actions (GetRecords, GetShardIterator, DescribeStream, ListStreams) — AWS rejects them with ValidationException: 'The following action names are invalid'. Streams actions are already granted via the identity-based policy on the MC kube-applier role (kube-applier/iam.tf) scoped to the RC account ARN. Remove the invalid AllowMCKubeApplierStreams statement from the specs resource policy; keep only the data-plane read actions there.
…ce-policy-v2 fix: remove invalid Streams actions from DynamoDB resource policy
The code uses attributeDocumentID = "documentID" (lowercase) throughout crud.go. The table was created with hash_key = "DocumentID" (capital D). DynamoDB attribute names are case-sensitive, so the Scan returned items with no readable partition key (empty DocumentID error) and GetItem failed with "key element does not match the schema". Fix both specs and status tables to use hash_key = "documentID" to match the application's constant. Note: existing tables must be recreated — DynamoDB does not allow changing the partition key of an existing table in-place.
…ention
Other platform tables follow the pattern {cluster-id}-{purpose} (e.g.
eph-e67cf96c-regional-authz-group-members) with no mc- prefix. The mc_name
already contains the mc identifier (e.g. eph-e67cf96c-mc01) so the prefix
was redundant.
Tables now named: {mc_name}-specs-{desire} / {mc_name}-status-{desire}
e.g. eph-e67cf96c-mc01-specs-applydesires
Also update deployment.yaml ARN prefix to remove the mc- segment.
kube-applier/iam.tf had table ARNs scoped to mc-{management_id}-specs-*
and mc-{management_id}-status-*. Now that the tables are named without the
mc- prefix, the identity-based policy must match.
…ion-key-case fix: use lowercase documentID as DynamoDB partition key
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
argocd/config/management-cluster/kube-applier/templates/deployment.yaml (1)
72-75: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winHarden the container securityContext.
The pod-level
securityContextsetsrunAsNonRootandseccompProfile, but thekube-appliercontainer has no container-levelsecurityContext. AddreadOnlyRootFilesystem: true,allowPrivilegeEscalation: false, and drop all capabilities to satisfy least-privilege hardening. Trivy KSV-0014 flags the missing read-only root filesystem.🛡️ Proposed container securityContext
containers: - name: kube-applier image: "{{ .Values.kubeApplier.image.registry }}/{{ .Values.kubeApplier.image.repository }}:{{ .Values.kubeApplier.image.tag }}" imagePullPolicy: {{ .Values.kubeApplier.image.pullPolicy }} + securityContext: + readOnlyRootFilesystem: true + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL args:Note: if the controller writes temp files, pair
readOnlyRootFilesystem: truewith anemptyDirmount.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml` around lines 72 - 75, The kube-applier workload currently only has pod-level hardening, but the container itself is missing a restrictive securityContext. Update the container spec for kube-applier to add readOnlyRootFilesystem, set allowPrivilegeEscalation to false, and drop all capabilities, while keeping the existing pod-level runAsNonRoot/seccompProfile settings. If the container needs writable temp space, add an emptyDir mount so the read-only filesystem setting can be enabled safely.Source: Linters/SAST tools
terraform/modules/kube-applier-dynamodb/main.tf (1)
61-62: 🎯 Functional Correctness | 🟠 MajorAdd resource policies for the specs table streams.
DynamoDB Streams require a resource-based policy on the stream ARN (not just the table ARN) to allow cross-account access. The current configuration enables streams but only attaches a policy to the table ARN. Without a separate
aws_dynamodb_resource_policytargetingstream_arn, cross-account principals (like the MC kube-applier role) will be deniedDescribeStream,GetRecords, andGetShardIteratorcalls even if their identity policy is correct.Suggested fix
resource "aws_dynamodb_resource_policy" "specs" { for_each = local.specs_tables resource_arn = aws_dynamodb_table.specs[each.key].arn @@ }) } + +resource "aws_dynamodb_resource_policy" "specs_streams" { + for_each = local.specs_tables + resource_arn = aws_dynamodb_table.specs[each.key].stream_arn + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Sid = "AllowMCKubeApplierStreamRead" + Effect = "Allow" + Principal = { + AWS = local.mc_kube_applier_role_arn + } + Action = [ + "dynamodb:DescribeStream", + "dynamodb:GetRecords", + "dynamodb:GetShardIterator", + ] + Resource = aws_dynamodb_table.specs[each.key].stream_arn + }] + }) +}This change aligns with AWS security best practices for
terraform/modules/**by ensuring least-privilege access is correctly structured for the enabled stream capabilities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/modules/kube-applier-dynamodb/main.tf` around lines 61 - 62, Add a separate resource-based policy for the DynamoDB stream, because the current specs table policy only covers the table ARN and does not authorize cross-account stream reads. Update the Terraform in the specs table setup so the existing stream-enabled configuration is paired with an aws_dynamodb_resource_policy that targets the stream_arn, and ensure it grants the MC kube-applier role the needed DescribeStream, GetRecords, and GetShardIterator permissions. Keep the table policy as-is for table access and use the stream-specific policy to cover stream access.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@argocd/config/management-cluster/kube-applier/templates/deployment.yaml`:
- Around line 72-75: The kube-applier workload currently only has pod-level
hardening, but the container itself is missing a restrictive securityContext.
Update the container spec for kube-applier to add readOnlyRootFilesystem, set
allowPrivilegeEscalation to false, and drop all capabilities, while keeping the
existing pod-level runAsNonRoot/seccompProfile settings. If the container needs
writable temp space, add an emptyDir mount so the read-only filesystem setting
can be enabled safely.
In `@terraform/modules/kube-applier-dynamodb/main.tf`:
- Around line 61-62: Add a separate resource-based policy for the DynamoDB
stream, because the current specs table policy only covers the table ARN and
does not authorize cross-account stream reads. Update the Terraform in the specs
table setup so the existing stream-enabled configuration is paired with an
aws_dynamodb_resource_policy that targets the stream_arn, and ensure it grants
the MC kube-applier role the needed DescribeStream, GetRecords, and
GetShardIterator permissions. Keep the table policy as-is for table access and
use the stream-specific policy to cover stream access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 24b35ed3-3c0c-4d6e-ae5c-0e7c8c9ecab1
📒 Files selected for processing (8)
argocd/config/management-cluster/kube-applier/templates/deployment.yamlargocd/config/management-cluster/kube-applier/values.yamlscripts/buildspec/dynamodb-mint.shterraform/config/kube-applier-dynamodb-provisioning/main.tfterraform/config/kube-applier-dynamodb-provisioning/variables.tfterraform/modules/kube-applier-dynamodb/main.tfterraform/modules/kube-applier-dynamodb/variables.tfterraform/modules/kube-applier/iam.tf
✅ Files skipped from review due to trivial changes (1)
- argocd/config/management-cluster/kube-applier/values.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- terraform/config/kube-applier-dynamodb-provisioning/main.tf
- terraform/modules/kube-applier/iam.tf
- scripts/buildspec/dynamodb-mint.sh
The stream watcher calls DescribeTable to resolve the stream ARN before opening shard iterators. This action was missing from both the MC role identity-based policy and the RC table resource-based policy, causing AccessDeniedException on startup in cross-account deployments."
For cross-account DynamoDB Streams access AWS requires the action to be permitted on both sides: the identity-based policy on the MC role AND the resource-based policy on the RC-account table. The previous comment claiming streams actions are not valid in resource-based policies was incorrect — DescribeStream/GetRecords/GetShardIterator/ListStreams must appear in both. The streams statement targets the stream sub-resource ARN (table/.../stream/*) rather than the table ARN itself.
Fixing permissions for streams
AWS strictly validates that actions in a resource-based policy are valid for the resource type the policy is attached to. Stream actions (DescribeStream, GetRecords, GetShardIterator, ListStreams) are invalid when attached to a table ARN and cause a ValidationException. Split into two aws_dynamodb_resource_policy resources: - specs: table ARN, table actions only (DescribeTable, GetItem, Scan, Query) - specs_stream: stream ARN, stream actions only
fix: split specs resource policy into separate table and stream policies
dynamodb:ListStreams operates at account/table scope and is invalid in a resource-based policy attached to a specific stream ARN — remove it from the specs_stream policy. It is already covered by the identity-based policy on the MC role. Also fix the deprecated data.aws_region.current.name reference in the kube-applier-dynamodb-provisioning config; .id is the current attribute. Co-authored-by: claude-sonnet-4-6 <claude-sonnet-4-6@anthropic.com>
The stream watcher resolves the stream ARN via DescribeTable, then calls DescribeStream, GetShardIterator, and GetRecords directly. ListStreams is never called and is not needed. Co-authored-by: claude-sonnet-4-6 <claude-sonnet-4-6@anthropic.com>
fix: remove ListStreams from stream policy; use .id for aws_region
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@terraform/modules/kube-applier-dynamodb/main.tf`:
- Around line 145-168: The new aws_dynamodb_resource_policy usage in
specs_stream requires a newer AWS provider, but the current version constraint
still allows unsupported releases. Update the Terraform provider constraint in
versions.tf for the kube-applier DynamoDB module to require AWS provider >=
5.43.0 so aws_dynamodb_resource_policy on the stream_arn can be planned and
applied successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9ee272c3-9dd5-4eca-84ee-9774803cdd43
📒 Files selected for processing (3)
terraform/config/kube-applier-dynamodb-provisioning/main.tfterraform/modules/kube-applier-dynamodb/main.tfterraform/modules/kube-applier/iam.tf
🚧 Files skipped from review as they are similar to previous changes (2)
- terraform/config/kube-applier-dynamodb-provisioning/main.tf
- terraform/modules/kube-applier/iam.tf
Comment on lines
+145
to
+168
| # Stream-level policy: stream read access for the MC kube-applier role. | ||
| # Attached to the stream ARN, not the table ARN, because stream actions are | ||
| # invalid in a table resource policy. | ||
| resource "aws_dynamodb_resource_policy" "specs_stream" { | ||
| for_each = local.specs_tables | ||
| resource_arn = aws_dynamodb_table.specs[each.key].stream_arn | ||
|
|
||
| policy = jsonencode({ | ||
| Version = "2012-10-17" | ||
| Statement = [{ | ||
| Sid = "AllowMCKubeApplierStreams" | ||
| Effect = "Allow" | ||
| Principal = { | ||
| AWS = local.mc_kube_applier_role_arn | ||
| } | ||
| Action = [ | ||
| "dynamodb:DescribeStream", | ||
| "dynamodb:GetRecords", | ||
| "dynamodb:GetShardIterator", | ||
| ] | ||
| Resource = aws_dynamodb_table.specs[each.key].stream_arn | ||
| }] | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the declared AWS provider version constraint for this module.
fd versions.tf terraform/modules/kube-applier-dynamodb --exec cat -n {}
# Also confirm the resource is used with a stream ARN.
rg -nP -C2 'aws_dynamodb_resource_policy|stream_arn' terraform/modules/kube-applier-dynamodb/main.tfRepository: openshift-online/rosa-hyperfleet
Length of output: 1251
🌐 Web query:
Which hashicorp/aws Terraform provider version added support for aws_dynamodb_resource_policy on DynamoDB stream ARNs?
💡 Result:
The aws_dynamodb_resource_policy resource was introduced in version 5.43.0 of the HashiCorp AWS Terraform provider [1][2]. This resource natively supports DynamoDB streams, as explicitly stated in the official Terraform documentation for aws_dynamodb_resource_policy, which notes that the resources you can specify include both tables and streams [3][4]. Since this functionality was part of the original implementation of the aws_dynamodb_resource_policy resource, it has been available since its release in provider version 5.43.0 [1][2].
Citations:
- 1: Amazon DynamoDB resource-based policies hashicorp/terraform-provider-aws#36502
- 2: r/dynamodb_resource_policy hashicorp/terraform-provider-aws#36595
- 3: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/dynamodb_resource_policy
- 4: https://registry.terraform.io/providers/hashicorp/aws/6.42.0/docs/resources/dynamodb_resource_policy
Update AWS provider version constraint to >= 5.43.0 to support aws_dynamodb_resource_policy on stream ARNs.
The aws_dynamodb_resource_policy resource, which is required here to attach a policy to the DynamoDB stream ARN, was introduced in the HashiCorp AWS provider version 5.43.0. The current constraint (>= 5.0) in terraform/modules/kube-applier-dynamodb/versions.tf allows versions prior to 5.43.0, which will cause terraform plan or apply to fail because the resource does not exist in those versions.
Update terraform/modules/kube-applier-dynamodb/versions.tf:
version = ">= 5.43.0"The policy logic attaching to the stream ARN is otherwise correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/modules/kube-applier-dynamodb/main.tf` around lines 145 - 168, The
new aws_dynamodb_resource_policy usage in specs_stream requires a newer AWS
provider, but the current version constraint still allows unsupported releases.
Update the Terraform provider constraint in versions.tf for the kube-applier
DynamoDB module to require AWS provider >= 5.43.0 so
aws_dynamodb_resource_policy on the stream_arn can be planned and applied
successfully.
Enables stream watcher logging at v=2 so stream connectivity and per-event delivery (documentID, eventType) are visible in pod logs without needing to restart with a manual flag. Co-authored-by: claude-sonnet-4-6 <claude-sonnet-4-6@anthropic.com>
chore: set kube-applier log verbosity to 2
…moDB provisioning - Add hyperfleet-operator chart and wire baseDomain/oidcIssuerBaseURL from Terraform through bootstrap to platform-api and operator - Consolidate fleet-db into RC VPC, removing separate DynamoDB pipeline stage - Move kube-applier DynamoDB provisioning to MC pipeline with cross-account access and DynamoDB Streams support - Remove maestro-agent, maestro-server, hyperfleet-adapter/sentinel/api charts and related IoT/AmazonMQ infrastructure (replaced by operator pattern) - Remove cluster-cleanup CronJob (replaced by operator reconciliation) - Add EKS workerless module for fleet-db cluster - Add retry and base64 fallback to ZOA runner ConfigMap patch - Fix MC registration payload alignment with platform-api contract Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: add hyperfleet-operator infra, fleet-db consolidation, and DynamoDB provisioning
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
typeid
changed the title
…unt kube-applier access The MC kube-applier role (in the MC account) was failing with AccessDeniedException when calling dynamodb:DescribeStream on specs tables in the RC account. For cross-account DynamoDB Streams access, AWS requires both an identity-based policy on the caller's role AND a resource-based policy on the stream resource itself. The stream ARN is a distinct resource type from the table — PutResourcePolicy supports it and the Streams actions (DescribeStream, GetRecords, GetShardIterator, ListStreams) all target the stream resource type. The previous comment claiming Streams actions are not valid in DynamoDB resource policies was incorrect. This adds aws_dynamodb_resource_policy.specs_stream (one per specs table) attached to each stream ARN, granting the MC kube-applier role the required Streams permissions cross-account.
ListStreams is an account-level action with no resource target and is rejected by DynamoDB when used in a stream resource-based policy. It remains covered by the identity-based policy on the MC kube-applier role (kube-applier/iam.tf).
…-resource-policy fix: add resource-based policy on DynamoDB stream ARNs for cross-account kube-applier access
The hyperfleet operator now watches streams on both status-applydesires and status-readdesires tables for event-driven reconciliation. Enable streams on all status tables and widen the IAM stream policy accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-ci
Bot
added
the
needs-rebase
|
@psav: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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
3 participants
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.
Summary by CodeRabbit
terraform.tfvarsfor DynamoDB provisioning and kube-applier.