add google identity federation #79
Conversation
WalkthroughThe pull request introduces changes to Terraform configurations for Google Cloud Platform (GCP) workload identity federation. A new secret variable for K3s JWKS (JSON Web Key Set) is added, and a new data block is created to access this secret. The workload identity pool provider is renamed, and a new Helm chart is deployed to manage workload identity federation webhook in the Kubernetes cluster. Changes
Sequence DiagramsequenceDiagram
participant SM as Secret Manager
participant WIF as Workload Identity Federation
participant K8s as Kubernetes Cluster
SM->>WIF: Retrieve JWKS Secret
WIF->>WIF: Configure Provider with JWKS
WIF->>K8s: Deploy Webhook
K8s-->>WIF: Webhook Installed
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![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
🔭 Outside diff range comments (1)
Terraform/GCP/iam/wif.tf (1)
Incorrect namespace restriction blocks CSI driver access
The attribute condition restricts workload identity federation to the "csi" namespace, but the GCP CSI driver is deployed in the "gce-pd-csi-driver" namespace. This will prevent the CSI driver from accessing GCP resources.
Update the attribute condition:
- attribute_condition = "attribute.kubernetes_namespace==\"csi\"" + attribute_condition = "attribute.kubernetes_namespace==\"gce-pd-csi-driver\""Also, consider implementing the security enhancements suggested in the original review for the OIDC configuration.
🔗 Analysis chain
Line range hint
14-27: Review the namespace restriction in attribute condition.The attribute condition
attribute.kubernetes_namespace=="csi"restricts workload identity federation to the "csi" namespace only. Verify if this is intentional or if other namespaces need access.Also, consider enhancing the OIDC configuration security:
oidc { issuer_uri = var.wif_issuer_uri - allowed_audiences = ["sts.googleapis.com"] + allowed_audiences = [var.workload_identity_audience] jwks_json = data.google_secret_manager_secret_version_access.k3s_jwks.secret_data } attribute_mapping = { "google.subject" = "assertion.sub" "attribute.kubernetes_namespace" = "assertion[\"kubernetes.io\"][\"namespace\"]" + "attribute.aud" = "assertion.aud" } - attribute_condition = "attribute.kubernetes_namespace==\"csi\"" + attribute_condition = "attribute.kubernetes_namespace==\"csi\" && attribute.aud==\"${var.workload_identity_audience}\""Run this script to verify namespace usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential workload identity users in other namespaces rg -l "serviceAccountName|workloadIdentity" | xargs rg "namespace:|serviceAccountName:"Length of output: 3029
🧹 Nitpick comments (2)
Terraform/GCP/k8s/wif.tf (1)
3-3: Consider using a version reference instead of a fixed chart path.Using a fixed path to a specific chart version makes updates harder to track and maintain. Consider:
- Moving the chart to a proper Helm repository
- Using a version variable for easier updates
- chart = "../../k8s/infrastructure/charts/gcp-workload-identity-federation-webhook-0.4.6.tgz" + repository = "https://your-helm-repo" + chart = "gcp-workload-identity-federation-webhook" + version = var.wif_webhook_versionTerraform/GCP/iam/variables.tf (1)
48-52: Add validation rules for the secret name variable.Consider adding validation rules to ensure the secret name follows GCP naming conventions.
variable "google_secret_name_k3s_jwks" { type = string description = "Google secret name for k3s jwks" default = "k3s-jwks" + validation { + condition = can(regex("^[a-zA-Z0-9-_]*$", var.google_secret_name_k3s_jwks)) + error_message = "Secret name must only contain alphanumeric characters, hyphens, and underscores." + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
k8s/infrastructure/charts/gcp-workload-identity-federation-webhook-0.4.6.tgzis excluded by!k8s/**/*.tgzand included byk8s/**
📒 Files selected for processing (3)
Terraform/GCP/iam/variables.tf(1 hunks)Terraform/GCP/iam/wif.tf(2 hunks)Terraform/GCP/k8s/wif.tf(1 hunks)
🔇 Additional comments (1)
Terraform/GCP/k8s/wif.tf (1)
1-9: Add configuration values for the Helm release.The Helm release lacks essential configuration values that might be needed for proper webhook operation:
- Resource requests and limits
- Webhook configuration parameters
- Logging settings
Please verify the required configuration by checking the chart's documentation:
✅ Verification successful
Basic Helm configuration is sufficient for the GCP WIF webhook
The current configuration is adequate because:
- The webhook's primary functionality relies on the GCP Workload Identity Federation setup, which is properly configured in
Terraform/GCP/iam/wif.tf- The chart (from pfnet-research) is designed to work with minimal configuration, automatically handling pod mutations based on the cluster's WIF configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Extract and check the chart documentation mkdir -p /tmp/chart && cd /tmp/chart tar xf ../../k8s/infrastructure/charts/gcp-workload-identity-federation-webhook-0.4.6.tgz cat */README.md */values.yamlLength of output: 430
Script:
#!/bin/bash # Search for the chart file and related documentation echo "=== Searching for chart files ===" fd -t f "gcp-workload-identity-federation-webhook.*\.tgz" || true echo -e "\n=== Searching for chart documentation ===" fd -t f "values\.ya?ml" -t f "README\.md" -t f "Chart\.ya?ml" | grep -i "workload.*identity\|wif" || true echo -e "\n=== Searching for example configurations ===" rg -l "gcp-workload-identity-federation-webhook" || trueLength of output: 755
Script:
#!/bin/bash # Search GitHub for the chart repository gh search repos "gcp-workload-identity-federation-webhook" --limit 5 || true echo -e "\n=== Checking if chart exists in common Helm repositories ===" helm search hub gcp-workload-identity-federation-webhook || true echo -e "\n=== Looking for any related configuration in the codebase ===" rg -A 5 -B 5 "workload.*identity" || trueLength of output: 2726
| data "google_secret_manager_secret_version_access" "k3s_jwks" { | ||
| project = var.project | ||
| secret = var.google_secret_name_k3s_jwks | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for secret access.
The secret access should be handled gracefully in case of failures. Consider using a lifecycle block to handle secret access errors.
data "google_secret_manager_secret_version_access" "k3s_jwks" {
project = var.project
secret = var.google_secret_name_k3s_jwks
+ lifecycle {
+ postcondition {
+ condition = self.secret_data != ""
+ error_message = "Failed to retrieve JWKS from Secret Manager"
+ }
+ }
}📝 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.
| data "google_secret_manager_secret_version_access" "k3s_jwks" { | |
| project = var.project | |
| secret = var.google_secret_name_k3s_jwks | |
| } | |
| data "google_secret_manager_secret_version_access" "k3s_jwks" { | |
| project = var.project | |
| secret = var.google_secret_name_k3s_jwks | |
| lifecycle { | |
| postcondition { | |
| condition = self.secret_data != "" | |
| error_message = "Failed to retrieve JWKS from Secret Manager" | |
| } | |
| } | |
| } |
* upgarde mysql 8.4.4 * edit new wi pool name * add google identity federation (#79)
Summary by CodeRabbit
Release Notes
New Features
Configuration Updates
The changes enhance security and identity management for Kubernetes deployments on Google Cloud Platform.