Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Bootstrap test integration and change in buckets creation #41

Merged

Conversation

mariammartins
Copy link
Collaborator

Hello folks,

This PR contains:

  1. The integration test for the bootstrap step, with the following resources:
  • Buckets
  • Source Repositories
  • Cloud Build Triggers
  • Service Accounts
  1. Add "location", "trigger_location" and "tf_apply_branches" variables in "tf_cloudbuild_workspace" module.

  2. Change in bucket creation:

  • Add the "bkt" prefix
  • Creation of a state bucket for each step and each environment (development, production and non-production)

@mariammartins mariammartins requested review from gtsorbo and a team as code owners March 11, 2024 13:08
@apeabody apeabody changed the title feat!: Bootstrat test integration and change in buckets creation feat!: Bootstrap test integration and change in buckets creation Mar 11, 2024
"github.com/stretchr/testify/assert"
"github.com/terraform-google-modules/terraform-example-foundation/test/integration/testutils"
Copy link
Collaborator

@apeabody apeabody Mar 11, 2024

Choose a reason for hiding this comment

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

IMHO the upstream isn't expecting external use, so I'd suggest we include the required function in this repo to minimize any future compatibility issues.

More ideally it would be added to blueprint-test utils

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are planning to move this function (and others we have on foundation) to this repo you mention.
We just didn't do yet to not block the PRs here.
But ASAP we will move all utils to toolkit.

@apeabody
Copy link
Collaborator

/gcbrun

1-bootstrap/main.tf Outdated Show resolved Hide resolved
@apeabody
Copy link
Collaborator

apeabody commented Mar 11, 2024

From the INT test:

TestBootstrap 2024-03-11T15:13:59Z command.go:100: Running command gcloud with args [source repos get-iam-policy eab-applicationfactory --flatten bindings --filter bindings.role:roles/viewer --format json]
TestBootstrap 2024-03-11T15:14:00Z command.go:185: ERROR: (gcloud.source.repos.get-iam-policy) PERMISSION_DENIED: The caller does not have permission
TestBootstrap 2024-03-11T15:14:00Z command.go:185: - '@type': type.googleapis.com/google.rpc.RequestInfo
TestBootstrap 2024-03-11T15:14:00Z command.go:185:   requestId: 8ff955bda0df4baf8de7be978eb5cb68

@Samir-Cit
Copy link
Collaborator

/gcbrun

Copy link
Collaborator

@Samir-Cit Samir-Cit left a comment

Choose a reason for hiding this comment

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

Made some comments on things that I think we can improve.

1-bootstrap/variables.tf Outdated Show resolved Hide resolved
1-bootstrap/variables.tf Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
test/integration/go.mod Outdated Show resolved Hide resolved
Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

LGTM - A few minor nits to resolve prior to merging

@gtsorbo - Ready for your review

1-bootstrap/backend.tf.example Outdated Show resolved Hide resolved
1-bootstrap/main.tf Outdated Show resolved Hide resolved
test/integration/bootstrap/bootstrap_test.go Outdated Show resolved Hide resolved
@mariammartins
Copy link
Collaborator Author

Hi, @gtsorbo
Could you take a look at the PR please? Whether it will be necessary to make any changes before merging. Thanks.

Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

LGTM, one nit

@gtsorbo, ready for your review

@@ -0,0 +1,31 @@
// Copyright 2022 Google LLC
Copy link
Collaborator

@apeabody apeabody Mar 25, 2024

Choose a reason for hiding this comment

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

Suggested change
// Copyright 2022 Google LLC
// Copyright 2024 Google LLC

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we fix this on the PR #61 ?
This build is green and ready to merge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure

@apeabody apeabody merged commit 6f5421e into GoogleCloudPlatform:main Mar 26, 2024
4 checks passed
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