Skip to content

feat: refactor ci cd repository - #1

Open
marcelorobj wants to merge 12 commits into
mainfrom
feat/refactor-secure-ci-cd
Open

feat: refactor ci cd repository#1
marcelorobj wants to merge 12 commits into
mainfrom
feat/refactor-secure-ci-cd

Conversation

@marcelorobj

@marcelorobj marcelorobj commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Description

This pull request significantly refactors the secure CI/CD pipeline to introduce support for 2nd-generation Cloud Build repository connections (GitHub and GitLab), adds a dedicated Binary Authorization attestor module, and implements VPC Service Controls (VPC-SC) compatibility. Additionally, it completely overhauls the integration testing framework to use a more modular, harness-based approach.

Specifically, the following changes were made:

  • Source Repository Removal: Updated the secure-ci and secure-cd modules to only support GitHub and GitLab repositories using Cloud Build 2nd-gen connections, controlled via the new repository_type, github_auth, and gitlab_auth variables.
  • New Attestor Module: Extracted and introduced a standalone attestor module (modules/attestor) to provision Binary Authorization attestors and their underlying Cloud KMS infrastructure.
  • VPC-SC Compatibility: Added the access_level_name variable across modules to conditionally apply IAM bindings to Access Context Manager, allowing the pipeline to operate securely within VPC Service Controls perimeters.
  • Integration Testing Overhaul: Completely refactored int.cloudbuild.yaml and the test/integration/ directory. Tests are now structured into modular harnesses (e.g., harness_gitlab, harness_vpcsc, harness_private_workerpool). Updated the Go version to 1.24.0 and the CFT developer tools Docker image to 1.25.
  • Automated Builder Image: The secure-ci module now automatically builds and pushes the custom Skaffold builder image to Artifact Registry during deployment using a null_resource.
  • VPN HA Bug Fix: Pinned the vpn_ha module version to != 2.3.2 and explicitly defined BGP session names to resolve a known bug with gateway peer configurations.
  • Examples Cleanup: Removed the deprecated private_cluster_cicd example and heavily updated the standalone_single_project example to demonstrate the new GitHub/GitLab capabilities and VPC-SC features, including an updated interactive walkthrough.

Related Issues/PRs

  • N/A

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g., README.md, metadata.yaml)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

How Has This Been Tested?

This refactor was thoroughly tested using the updated Cloud Foundation Toolkit (CFT) blueprint testing framework.

  • Executed the new harness-based integration tests (TestGitLab, TestVPCSC, TestPrivateWorkerPool, TestLoggingBucket) via int.cloudbuild.yaml to verify individual component provisioning.
  • Verified the automated GitLab VM bootstrapping and startup scripts using TestBootstrapGitlabVM and TestValidateStartupScript.
  • Deployed the standalone_single_project example locally using both GITHUB and GITLAB repository types to ensure end-to-end trigger creation and IAM bindings succeed.
  • Confirmed that the vpn_ha module successfully creates BGP sessions with the pinned module version.

Important Notes

  • New Prerequisites: Utilizing the new GitHub/GitLab integrations requires enabling the Secret Manager API (secretmanager.googleapis.com) and granting the deploying identity roles/cloudbuild.connectionAdmin and roles/secretmanager.admin.
  • Removed Example: The private_cluster_cicd example has been permanently removed in favor of consolidating private cluster and VPC-SC demonstrations into the standalone_single_project example.

cloud-foundation-bot and others added 3 commits June 8, 2026 17:07
feat: refactoring secure cicd repo

feat: adding gitlab vm for integration test

feat: adding gitlab vm for integration test
@marcelorobj marcelorobj changed the title Feat/refactor secure ci cd feat: refactor ci cd repository Jun 8, 2026

@marcos-leal-cit marcos-leal-cit left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some format adjusts.

I have noticed that there are some new files with the copyright in referencing different years:

// Copyright 2024 Google LLC
// Copyright 2025 Google LLC
// Copyright 2024-2025 Google LLC

If it makes sense, double-check if it is reasonable to change the year to 2026.

By the way, the lint is also indicating a failure. Take a look to see what happened.

Comment thread build/int.cloudbuild.yaml

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have noticed that in some cases you define the args in single line, and other cases in multiple ones.
For example:

args:
      ["/bin/bash", "-c", "cft test run TestPrivateWorkerPool --stage init --verbose"]

And multiple lines, for example:

    args:
      [
        "/bin/bash",
        "-c",
        "source /usr/local/bin/task_helper_functions.sh && prepare_environment",
      ]

Except for those cases who has a long instruction, such as the - id: bootstrap-gitlab-vm, it would be good to have a single pattern.

version = "~> 1.0"

project_id = var.project_id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT, remove spaces between module variables.

}
]

# --- START OF THE CORRECT FIX ---

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debug comments.

value = module.cd_pipeline.deploy_trigger_names
}

output "gar_repo_name" {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand, the name of "gar" is because of "Google Artifact Registry"? If yes, I recommend to also adjust the description for "Name of the Google Artifact Registry repository"

repository_name = string
repository_url = string
})
description = "The CI repository to configure. The key is a short name for the service."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review description, referencing CI on a CD variable.

Comment thread test/setup/main.tf Outdated
Comment on lines +146 to +150
resource "google_organization_iam_member" "int_test_access_context_manager_admin" {
org_id = var.org_id
role = "roles/accesscontextmanager.policyAdmin"
member = "serviceAccount:${google_service_account.int_test.email}"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This resource seems duplicated, it already has a iam_member for the same role and member under test/setup/iam.tf

Comment thread test/setup/versions.tf Outdated
}
time = {
source = "hashicorp/time"
version = ">= 0.12.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here regarding major version control.

Comment thread test/setup/versions.tf Outdated
}
random = {
source = "hashicorp/random"
version = ">= 3.7.2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here regarding major version control.

Comment thread metadata.yaml
- roles/iam.serviceAccountAdmin
- roles/serviceusage.serviceUsageViewer
- roles/iam.serviceAccountUser
- level: Project

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here regarding -level: Project roles, it seems duplicated. I suggest to double-check.

Comment thread README.md Outdated
Comment on lines +15 to +25
1. A developer pushes new code or a code change for a container-based application to a Source Repository (Cloud Source Repositories, GitHub, or GitLab).
2. The code push invokes a Cloud Build trigger. The Cloud Build trigger starts a build in a Cloud Build private worker pool that's hosted in a customer-managed VPC. The outputs of the build are metadata files, Cloud Build logs, and containers.
3. The metadata files and the Cloud Build logs are stored in a Cloud Storage bucket.
4. The pipeline runs security scans (which you configure) and validates the container structure. When the scans and structure pass, the containers are stored in Artifact Registry.
5. The Cloud Build trigger requests an attestation from Binary Authorization that certifies that the required scans passed. The attestation is stored as a cryptographic signature in Binary Authorization.
6. The Cloud Build trigger starts a Cloud Deploy release to roll out the containers to the three environments: developer (Dev), QA, and production (Prod). Each environment is one Kubernetes cluster in its own subnet. All three clusters and subnets are in the same GKE VPC. The last cluster is the production environment.
7. As the rollout starts, Cloud Deploy sends the containers to the developer environment. Google Kubernetes Engine (GKE) uses the policy that's defined for the cluster to check the containers’ build attestation in Binary Authorization. When this check passes, GKE deploys the containers into the Kubernetes cluster.
8. When Cloud Deploy releases the containers to the developer environment, Cloud Deploy sends a Pub/Sub message that starts the second Cloud Build trigger. This Cloud Build trigger runs post-deployment tests (which you configure) in the developer environment. To run these tests, Cloud Build worker pools communicate with the clusters using Connect Gateway.
9. When the post-deployment checks succeed, the Cloud Build trigger requests an attestation from Binary Authorization. The attestation certifies that the required tests from the developer environment passed.
10. Cloud Deploy promotes the release to the second Kubernetes cluster for the QA environment. Steps 7 to 9 run again with some differences: GKE checks for the two attestations before deploying, and after the tests pass, the quality attestation is created.
11. Cloud Deploy promotes the release to the production environment, which is the third Kubernetes cluster. GKE uses a policy to check for all three attestations in Binary Authorization. When this check passes, GKE deploys the containers in the Kubernetes cluster for the production environment.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In markdown files, it is usually used "1." in a numbered list because when on the viewer it automatically transforms in the sequence "1., 2., 3., ...". It is also easier to maintain, since if something change in the middle, the user does not need to enumerate everything all over again.
I suggest to keep numbered list as "1." as before.


variable "private_worker_pool_id" {
description = "Optional private worker pool id if using already existing worker pool"
default = null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
default = null
default = null
validation {
condition = var.private_worker_pool_id != ""
error_message = "private_worker_pool_id cannot be empty, only null or a valid value.
}
validation {
condition = var.private_worker_pool_id == null ? true : can(regex("^projects/[a-z0-9-]+/locations/[a-z0-9-]+/workerPools/[a-z0-9-]+$", var.private_worker_pool_id))
error_message = "The private_worker_pool_id must follow the exact format: 'projects/PROJECT/locations/LOCATION/workerPools/POOL_NAME'."
}

)

// Retry retries a function a given number of times with a delay between attempts.
func Retry(retries int, delay time.Duration, f func() error) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this function be embedded in the cft framework?

* limitations under the License.
*/

module "private_workerpool_project" {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have two modules for private worker pool? one in /modules e other in /test/setup/modules?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants