Argocdcloud#35
Conversation
There was a problem hiding this comment.
Pull request overview
Establishes a multi-cloud infrastructure-as-code baseline with governance policy enforcement, CI/CD automation, and GitOps-managed Kubernetes deployments via ArgoCD.
Changes:
- Introduces per-cloud Terraform entrypoints (aws/azure/gcp) and standardizes module/provider versioning and backends.
- Expands OPA/Conftest policy packs for Terraform + Kubernetes and integrates them into a matrix GitHub Actions pipeline.
- Adds GitOps application manifests (ArgoCD), Kubernetes “app” and “test-app” workloads, and repo automation (Dependabot, label sync, pre-commit, EditorConfig).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| policies/terraform/security.rego | Extends Terraform governance/security policies (tags, encryption, flow logs, etc.). |
| policies/terraform/sample-tfplan.json | Updates sample plan to include module-scoped addresses and flow log resource. |
| policies/kubernetes/required-labels.rego | Switches to Rego v1 + broadens Kubernetes governance rules (resources, probes, security). |
| k8s/test-app/namespace.yaml | Adds a dedicated namespace for smoke-test workload. |
| k8s/test-app/deployment.yaml | Adds hardened Deployment for test workload (resources/probes/securityContext). |
| k8s/test-app/service.yaml | Adds Service for test workload. |
| k8s/test-app/hpa.yaml | Adds HPA for test workload. |
| k8s/test-app/pdb.yaml | Adds PDB for test workload. |
| k8s/app/namespace.yaml | Adds primary application namespace manifest. |
| k8s/app/deployment.yaml | Hardens primary Deployment (resources/probes/securityContext/volumes). |
| k8s/app/hpa.yaml | Adds HPA for primary workload. |
| k8s/app/pdb.yaml | Adds PDB for primary workload. |
| infra/terraform/main.tf | Removes prior “single-entry” multi-cloud router module approach. |
| infra/terraform/providers.tf | Removes root provider configuration (now cloud-specific). |
| infra/terraform/versions.tf | Removes root provider/version constraints (now per directory). |
| infra/terraform/outputs.tf | Removes root outputs (now per cloud directory). |
| infra/terraform/backend.tf.example | Documents backend config moved to cloud-specific directories. |
| infra/terraform/terraform.tfvars.example | Provides example tfvars for per-cloud usage. |
| infra/terraform/aws/main.tf | Adds AWS entrypoint wiring to module with common tags. |
| infra/terraform/aws/providers.tf | Adds AWS provider config in AWS directory. |
| infra/terraform/aws/variables.tf | Adds AWS variables (region, tags, CIDRs, etc.). |
| infra/terraform/aws/outputs.tf | Adds AWS outputs for network/subnet and common metadata. |
| infra/terraform/aws/versions.tf | Pins Terraform + AWS provider versions for AWS directory. |
| infra/terraform/aws/backend.tf.example | Adds S3 backend example for AWS. |
| infra/terraform/azure/main.tf | Adds Azure entrypoint wiring to module with common tags. |
| infra/terraform/azure/providers.tf | Adds AzureRM provider config in Azure directory. |
| infra/terraform/azure/variables.tf | Adds Azure variables (location, tags, CIDRs, etc.). |
| infra/terraform/azure/outputs.tf | Adds Azure outputs for network/subnet and common metadata. |
| infra/terraform/azure/versions.tf | Pins Terraform + AzureRM provider versions for Azure directory. |
| infra/terraform/azure/backend.tf.example | Adds Azure Storage backend example. |
| infra/terraform/gcp/main.tf | Adds GCP entrypoint wiring to module with common tags locals. |
| infra/terraform/gcp/providers.tf | Adds Google provider config in GCP directory. |
| infra/terraform/gcp/variables.tf | Refactors GCP variables; removes cross-cloud variables from GCP dir. |
| infra/terraform/gcp/outputs.tf | Adds GCP outputs for network/subnet and common metadata. |
| infra/terraform/gcp/versions.tf | Pins Terraform + Google provider versions for GCP directory. |
| infra/terraform/gcp/backend.tf.example | Adds GCS backend example for GCP. |
| infra/terraform/modules/aws/main.tf | Adds VPC flow logs resources to AWS module. |
| infra/terraform/modules/aws/variables.tf | Documents AWS module inputs. |
| infra/terraform/modules/aws/versions.tf | Pins Terraform + AWS provider versions for AWS module. |
| infra/terraform/modules/azure/variables.tf | Documents Azure module inputs. |
| infra/terraform/modules/azure/versions.tf | Pins Terraform + AzureRM provider versions for Azure module. |
| infra/terraform/modules/gcp/main.tf | Defines GCP network/subnetwork resources and outputs. |
| infra/terraform/modules/gcp/variables.tf | Documents GCP module inputs and interface consistency. |
| infra/terraform/modules/gcp/versions.tf | Pins Terraform + Google provider versions for GCP module. |
| argocd/application.yaml | Fixes repo reference/branch and adds retry policy for sync. |
| argocd/test-application.yaml | Adds separate ArgoCD Application for test workload with retry/backoff. |
| .github/workflows/pipeline.yml | Adds multi-cloud matrix CI (plan + OPA + Conftest + artifacts). |
| .github/workflows/labels.yml | Adds label sync workflow. |
| .github/labels.yml | Adds repository label definitions for automation. |
| .github/dependabot.yml | Configures Dependabot for GitHub Actions + Terraform directories. |
| .pre-commit-config.yaml | Adds pre-commit hooks for formatting/validation/policy checks. |
| .editorconfig | Enforces consistent formatting across file types. |
| .gitignore | Ignores Terraform state/plan artifacts, IDE files, and env/key material. |
| Makefile | Adds targets for Terraform, policies, kubectl dry runs, and ArgoCD workflows. |
| README.md | Expands documentation for architecture, usage, policies, CI, GitOps, and ops. |
| LICENSE | Adds MIT license text for open-source compliance. |
Comments suppressed due to low confidence (3)
.github/workflows/pipeline.yml:31
- This
terraform fmt -checkonly runs within the cloud-specific directory, so formatting issues insideinfra/terraform/modules/**won’t be caught by CI. Consider running the check oninfra/terraform(or add an additional fmt check forinfra/terraform/modules) so the pipeline consistently enforces formatting across all Terraform code.
- name: Terraform format check
run: terraform fmt -check -recursive infra/terraform/${{ matrix.cloud }}
policies/kubernetes/required-labels.rego:77
- These probe rules are enforced via
deny, but the message says "should". This is confusing for users because it reads like a recommendation while CI will treat it as a hard failure. Align the message wording with the enforcement level (or switch to an advisory result if that’s the intent).
# Check for liveness and readiness probes
deny contains msg if {
obj := input
obj.kind == "Deployment"
container := obj.spec.template.spec.containers[_]
not container.livenessProbe
msg := sprintf("Deployment/%s: container '%s' should define a livenessProbe", [obj.metadata.name, container.name])
}
deny contains msg if {
obj := input
obj.kind == "Deployment"
container := obj.spec.template.spec.containers[_]
not container.readinessProbe
msg := sprintf("Deployment/%s: container '%s' should define a readinessProbe", [obj.metadata.name, container.name])
}
.github/workflows/pipeline.yml:11
- Using
OPA_VERSION: latestmakes CI non-reproducible and can introduce breaking policy evaluation changes without any code change. Pin OPA to a specific version (similar to Terraform/Conftest) and update it intentionally via Dependabot or a scheduled bump.
env:
TERRAFORM_VERSION: 1.8.5
OPA_VERSION: latest
CONFTEST_VERSION: 0.56.0
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
policies/kubernetes/required-labels.rego:16
- This policy imports
rego.v1but the required-labels rule is still written using the olderdeny[msg] { ... }syntax, while the rest of the file usesdeny contains msg if. To avoid mixed semantics (and potentialopa check/Conftest parsing issues), rewrite this rule using the samedeny contains msg if { ... }form (or dropimport rego.v1if you intend to keep v0 rule syntax).
deny[msg] {
obj := input
kind := object.get(obj, "kind", "")
kind != ""
labels := object.get(object.get(obj, "metadata", {}), "labels", {})
missing := required_labels - object.keys(labels)
count(missing) > 0
name := object.get(object.get(obj, "metadata", {}), "name", "unknown")
msg := sprintf("%s/%s is missing required labels: %v", [kind, name, missing])
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@stephenjtyrrell I've opened a new pull request, #36, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| argocd-test: ## Apply app and wait until ArgoCD reports Synced + Healthy | ||
| kubectl apply -f $(ARGOCD_APP_MANIFEST) | ||
| kubectl wait --for=jsonpath='{.status.sync.status}'=Synced application/$(ARGOCD_APP_NAME) -n $(ARGOCD_NAMESPACE) --timeout=300s | ||
| kubectl wait --for=jsonpath='{.status.health.status}'=Healthy application/$(ARGOCD_APP_NAME) -n $(ARGOCD_NAMESPACE) --timeout=300s | ||
| kubectl get application -n $(ARGOCD_NAMESPACE) $(ARGOCD_APP_NAME) | ||
| kubectl get all -n dissertation |
There was a problem hiding this comment.
argocd-test hard-codes the application namespace (dissertation) for the final kubectl get all, while other values are parameterized (e.g., ARGOCD_NAMESPACE, app name, manifest path). This makes the target brittle if the destination namespace changes. Make the destination namespace configurable (e.g., APP_NAMESPACE?=dissertation) and use it here, similar to how the test app uses dissertation-test.
| rm -f $(TF_DIR)/aws/*.tfplan $(TF_DIR)/aws/tfplan $(TF_DIR)/aws/tfplan*.json $(TF_DIR)/azure/*.tfplan $(TF_DIR)/azure/tfplan $(TF_DIR)/azure/tfplan*.json $(TF_DIR)/gcp/*.tfplan $(TF_DIR)/gcp/tfplan $(TF_DIR)/gcp/tfplan*.json | ||
| rm -rf $(TF_DIR)/aws/.terraform $(TF_DIR)/azure/.terraform $(TF_DIR)/gcp/.terraform | ||
| rm -f $(TF_DIR)/aws/.terraform.lock.hcl $(TF_DIR)/azure/.terraform.lock.hcl $(TF_DIR)/gcp/.terraform.lock.hcl |
There was a problem hiding this comment.
The clean target duplicates provider paths and filenames, which will be easy to break (or forget to update) as more clouds/dirs are added. Prefer a small loop over $(CLOUDS) (e.g., aws azure gcp), or a find-based cleanup scoped to $(TF_DIR) to remove tfplan*, .terraform/, and .terraform.lock.hcl per subdirectory.
| package main | ||
|
|
||
| import future.keywords.contains | ||
| import future.keywords.if |
There was a problem hiding this comment.
This file now mixes newer deny contains msg if { ... } rules with older deny[msg] { ... } rules (seen earlier in the file per the diff context). That works, but it reduces consistency and can confuse future edits—especially since you're importing future.keywords.contains/if. Consider converting the remaining deny[msg] { ... } rules to the same deny contains msg if { ... } style (or reverting to a single style), and drop import future.keywords.if if it’s not actually used anywhere in the policy.
| # Check for resource limits on containers | ||
| deny[msg] { | ||
| deny contains msg if { |
There was a problem hiding this comment.
This file now mixes newer deny contains msg if { ... } rules with older deny[msg] { ... } rules (seen earlier in the file per the diff context). That works, but it reduces consistency and can confuse future edits—especially since you're importing future.keywords.contains/if. Consider converting the remaining deny[msg] { ... } rules to the same deny contains msg if { ... } style (or reverting to a single style), and drop import future.keywords.if if it’s not actually used anywhere in the policy.
This pull request introduces a comprehensive set of foundational improvements for infrastructure-as-code, CI/CD automation, and repository management. It adds initial Terraform configurations for AWS and Azure, sets up policy enforcement and pre-commit hooks, and automates dependency and label management. These changes establish best practices for multi-cloud infrastructure, policy compliance, and continuous integration.
Infrastructure as Code:
main.tf,variables.tf,outputs.tf,providers.tf,versions.tf, and backend configuration examples. These modules define networking resources and enforce tagging standards. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]CI/CD and Policy Enforcement:
.pre-commit-config.yamlto enforce code formatting, validate Terraform and YAML files, and verify policy files on commit.Repository Management and Automation:
.editorconfigfor consistent code style and formatting across file types.Makefilewith targets for Terraform, policy checks, Kubernetes dry runs, ArgoCD app management, and cleanup, supporting multi-cloud workflows.LICENSEfile under the MIT license for open source compliance.GitHub Automation:
label-syncaction. [1] [2]ArgoCD Application Management: