Add cert-manager root certificate #337
Conversation
WalkthroughThis pull request updates several Terraform and pre-commit configuration files. It modifies versions for various providers, including the Changes
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
💰 Infracost reportMonthly estimate generatedEstimate details (includes details of unsupported resources and skipped projects due to errors) |
brettcurtis
temporarily deployed
to
Sandbox: Main
GitHub Actions
Inactive
brettcurtis
had a problem deploying
to
Sandbox: us-east1-b
GitHub Actions
Error
brettcurtis
had a problem deploying
to
Sandbox: us-east4-a
GitHub Actions
Error
brettcurtis
requested a deployment
to
Sandbox: us-east1-b
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox: us-east4-a
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox: us-east4-a
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox: us-east1-b
GitHub Actions
Waiting
… and add remote state datasource
brettcurtis
temporarily deployed
to
Sandbox: Main
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox: us-east4-a
GitHub Actions
Inactive
brettcurtis
had a problem deploying
to
Sandbox: us-east1-b
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
regional/cert-manager/istio-csr/README.md (1)
30-37: Documentation Discrepancy in Outputs Section
The tf-docs block under the Outputs section still shows "No outputs." even though new outputs for the cert-manager root certificate and key have been added (as seen inoutputs.tf). Consider updating this section so that the documentation accurately reflects all available outputs.README.md (1)
Line range hint
60-64: Cert-Manager Module & Outputs Documentation AddedThe README now documents the new
kubernetes_cert_managermodule sourced from "github.com/osinfra-io/terraform-kubernetes-cert-manager" with the version tag "tls". Additionally, two new output entries provide details for the cert-manager root certificate and its private key.Recommendation: Confirm that in the corresponding Terraform module the private key output is marked as sensitive to prevent accidental exposure.
Also applies to: 87-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.pre-commit-config.yaml(2 hunks).terraform.lock.hcl(2 hunks)README.md(2 hunks)main.tf(1 hunks)outputs.tf(1 hunks)regional/cert-manager/.terraform.lock.hcl(1 hunks)regional/cert-manager/README.md(1 hunks)regional/cert-manager/istio-csr/.terraform.lock.hcl(1 hunks)regional/cert-manager/istio-csr/README.md(2 hunks)regional/cert-manager/istio-csr/locals.tf(1 hunks)regional/cert-manager/istio-csr/main.tf(1 hunks)regional/cert-manager/istio-csr/variables.tf(1 hunks)regional/datadog/.terraform.lock.hcl(1 hunks)regional/datadog/README.md(1 hunks)regional/datadog/manifests/.terraform.lock.hcl(1 hunks)regional/datadog/manifests/README.md(1 hunks)regional/istio/.terraform.lock.hcl(1 hunks)regional/istio/README.md(1 hunks)regional/istio/manifests/.terraform.lock.hcl(1 hunks)regional/istio/manifests/README.md(1 hunks)regional/istio/test/README.md(1 hunks)regional/onboarding/.terraform.lock.hcl(1 hunks)regional/onboarding/README.md(1 hunks)regional/opa-gatekeeper/.terraform.lock.hcl(1 hunks)regional/opa-gatekeeper/README.md(1 hunks)regional/opa-gatekeeper/manifests/.terraform.lock.hcl(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- regional/datadog/manifests/README.md
- regional/istio/manifests/README.md
- regional/cert-manager/istio-csr/locals.tf
- regional/istio/README.md
- regional/opa-gatekeeper/README.md
- regional/onboarding/README.md
- regional/datadog/README.md
- regional/cert-manager/README.md
- regional/istio/test/README.md
- regional/opa-gatekeeper/.terraform.lock.hcl
- regional/opa-gatekeeper/manifests/.terraform.lock.hcl
- regional/onboarding/.terraform.lock.hcl
🧰 Additional context used
📓 Learnings (1)
regional/cert-manager/istio-csr/variables.tf (1)
Learnt from: brettcurtis
PR: osinfra-io/google-cloud-kubernetes#150
File: regional/cert-manager/istio-csr/tfvars/us-east4-c-sandbox.tfvars:2-2
Timestamp: 2024-11-12T12:23:57.379Z
Learning: In Terraform `.tfvars` files, the `remote_bucket` variable is intentionally named differently across all environments.
🪛 Checkov (3.2.334)
main.tf
[HIGH] 18-20: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Regional: us-east1-b / Terraform apply
- GitHub Check: Regional: us-east4-a / Terraform apply
🔇 Additional comments (20)
regional/cert-manager/istio-csr/variables.tf (1)
4-7: New Input Variableremote_bucketDefined Correctly
The addition of theremote_bucketvariable is clear and follows Terraform best practices. The description properly explains its purpose for configuring the remote state data source.regional/cert-manager/istio-csr/main.tf (2)
1-13: Add Remote State Data Source Configuration
The newdata "terraform_remote_state" "main"block is correctly configured:
- Uses the
gcsbackend.- Dynamically references the bucket via
var.remote_bucketand the prefix viamodule.helpers.repository.- Constructs the workspace name using
module.helpers.environment.Ensure that the referenced outputs (e.g. from
module.helpers) are available.
19-24: Module Source and TLS Variables Update
The update in the module block forkubernetes_cert_manager_istio_csris well done:
- The source has been updated from a fixed version to use
ref=tls, increasing flexibility.- New variables for the cert-manager root certificate and key (
tls_self_signed_cert_cert_manager_root_certandtls_self_signed_cert_cert_manager_root_key) have been added and are sourced from local values.Double-check that the local variables (
local.main.kubernetes_cert_manager_tls_self_signed_cert_cert_manager_root_certand..._root_key) are properly defined elsewhere..pre-commit-config.yaml (2)
15-15: Pre-commit Terraform Hook Revision Update
The update torev: v1.97.1for the pre-commit-terraform hooks is in line with keeping dependencies current. Verify that this change does not introduce any incompatibilities with your existing Terraform configurations.
36-43: Checkov Version and Skipped Checks Adjusted
Updating the Checkov revision to3.2.360and modifying the skip-check argument to["CKV_TF_1,CKV_TF_2"]is clear. Just ensure that excluding both check IDs is intentional and that it does not mask potential issues in your Terraform code.outputs.tf (2)
14-17: New Output for Cert-Manager Root Certificate
The newly added output forkubernetes_cert_manager_tls_self_signed_cert_cert_manager_root_certis correctly defined and provides clear documentation.
19-24: New Output for Cert-Manager Root Key with Sensitivity Flag
The output for the cert-manager root key is properly marked as sensitive. Confirm that the module indeed produces the expected values for these outputs.regional/cert-manager/istio-csr/README.md (3)
12-13: Provider Versions Updated in Documentation
The documentation now reflects the updated provider versions—6.18.1and a newterraformprovider is introduced with versionn/a. This aligns well with the overall version bump effort.
20-20: Module Version Adjustment Documented
Changing the module version forkubernetes_cert_manager_istio_csrtotlsis clearly documented. Ensure that stakeholders are aware of any behavioral differences compared to version-specific tags.
34-36: New Input Documentation forremote_bucket
The README now includes details for the newremote_bucketinput, matching its definition invariables.tf.main.tf (2)
22-31: Module "kubernetes_engine": Confirm configuration compliance.The module is sourced with a pinned version using
?ref=v0.2.2and its inputs (e.g.
namespacesandproject) are correctly provided. Verify that this configuration is consistent with the overall infrastructure strategy.
32-41: Module "kubernetes_istio": Validate input variable mapping.The module is properly pinned with
?ref=v0.1.7and the variables like
gateway_dns,labels, andprojectare passed as expected. Confirm that these inputs align with your deployment requirements for Istio.regional/datadog/manifests/.terraform.lock.hcl (1)
5-16: Google Provider Version Update in Manifests Lock File.The provider
"registry.terraform.io/hashicorp/google"has been updated to version"6.18.1"with a refreshed set of hash values. Ensure that these new hashes match those officially provided for the release to safeguard against integrity issues.regional/datadog/.terraform.lock.hcl (1)
5-16: Google Provider Version Update in Datadog Lock File.The update to version
"6.18.1"along with the new hash values for the Google provider is consistent with repository-wide changes. Please verify the correctness of these new hash values to maintain Terraform integrity.regional/istio/manifests/.terraform.lock.hcl (1)
5-18: Google Provider Update in Istio Manifests Lock File.The provider version has been updated to
"6.18.1"and all associated hash values have been replaced. This change standardizes the provider version across environments. Confirm that these updates are validated against the official provider release data.regional/cert-manager/.terraform.lock.hcl (1)
5-18: Google Provider Update in Cert-Manager Lock File.The provider
"registry.terraform.io/hashicorp/google"now uses version"6.18.1", with its hash values completely replaced. This update is consistent with the other regions. Ensure that the new hashes are correct and that they align with the release documentation for the provider.regional/cert-manager/istio-csr/.terraform.lock.hcl (1)
5-18: Google Provider Version & Hashes UpdatedThe Google provider is now set to version "6.18.1" with a refreshed list of hash values. This update is in line with similar changes elsewhere in the repository. Please make sure that these new hashes are validated (e.g., via a fresh
terraform init) to ensure consistency across your environments.regional/istio/.terraform.lock.hcl (1)
5-18: Consistent Google Provider Update in Istio ConfigurationThe update to version "6.18.1" for the Google provider and the corresponding new hashes are properly reflected in this lock file. This change appears consistent with updates in other regional configurations.
.terraform.lock.hcl (2)
45-59: Google-Beta Provider UpdateThe
google-betaprovider block now uses version "6.18.1" and includes a new set of hash values. This change aligns with the upgrade of the stable Google provider, ensuring uniformity across provider versions.
81-99: New TLS Provider Block AddedA new provider block for
hashicorp/tls(version "4.0.6") has been introduced. This addition is likely required to support the cert-manager root certificate functionality. Verify that all necessary configurations (e.g., in your cert-manager module) are updated accordingly.
| # Kubernetes cert-manager Module (osinfra.io) | ||
| # https://github.com/osinfra-io/terraform-kubernetes-cert-manager | ||
|
|
||
| module "kubernetes_cert_manager" { | ||
| source = "github.com/osinfra-io/terraform-kubernetes-cert-manager?ref=tls" | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Module "kubernetes_cert_manager": Validate module source versioning.
The module source is defined as
source = "github.com/osinfra-io/terraform-kubernetes-cert-manager?ref=tls"
which uses the reference tls rather than a semantic version number. This could lead to unexpected changes if the branch is updated. Consider pinning to a specific release tag to ensure stability.
🧰 Tools
🪛 Checkov (3.2.334)
[HIGH] 18-20: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
brettcurtis
had a problem deploying
to
Sandbox Onboarding: us-east4-a
GitHub Actions
Error
brettcurtis
temporarily deployed
to
Sandbox: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Onboarding: us-east4-a
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Onboarding: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Datadog: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox cert-manager: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Datadog: us-east4-a
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox cert-manager: us-east4-a
GitHub Actions
Inactive
brettcurtis
requested a deployment
to
Sandbox Datadog Manifests: us-east4-a
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox Datadog Manifests: us-east1-b
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox OPA Gatekeeper: us-east1-b
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox OPA Gatekeeper: us-east4-a
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox cert-manager: us-east1-b
GitHub Actions
Waiting
brettcurtis
temporarily deployed
to
Sandbox cert-manager Istio CSR: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox OPA Gatekeeper: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Istio: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Istio: us-east4-a
GitHub Actions
Inactive
brettcurtis
deployed
to
Sandbox Istio Manifests: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Istio Manifests: us-east1-b
GitHub Actions
Active
brettcurtis
temporarily deployed
to
Sandbox Istio Test: us-east4-a
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Istio Test: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Onboarding: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox Onboarding: us-east4-a
GitHub Actions
Inactive
brettcurtis
requested a deployment
to
Sandbox Datadog: us-east4-a
GitHub Actions
Waiting
brettcurtis
requested a deployment
to
Sandbox Datadog: us-east1-b
GitHub Actions
Waiting
brettcurtis
temporarily deployed
to
Sandbox cert-manager Istio CSR: us-east1-b
GitHub Actions
Inactive
brettcurtis
temporarily deployed
to
Sandbox cert-manager Istio CSR: us-east4-a
GitHub Actions
Inactive
brettcurtis
deployed
to
Sandbox cert-manager: us-east1-b
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox cert-manager Istio CSR: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox OPA Gatekeeper: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Istio: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox cert-manager: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Istio Test: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Istio Test: us-east1-b
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox cert-manager Istio CSR: us-east1-b
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox OPA Gatekeeper: us-east1-b
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Istio: us-east1-b
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Onboarding: us-east4-a
GitHub Actions
Active
brettcurtis
deployed
to
Sandbox Onboarding: us-east1-b
GitHub Actions
Active
brettcurtis
had a problem deploying
to
Sandbox: us-east1-b
GitHub Actions
Failure
Summary by CodeRabbit
New Features
remote_bucketacross multiple environments for remote storage configuration.Refactor
Chores
Documentation