Fix/update acm asm - Githubissues

GoogleCloudPlatform / terraform-example-foundation-app

https://registry.terraform.io/modules/GoogleCloudPlatform/terraform-example-foundation-app/google

Apache License 2.0

33 stars 36 forks source link

Fix/update acm asm #50

Closed dkassab closed 3 years ago

dkassab commented 3 years ago

Updating documentation and fixing formatting issues.

daniel-cit commented 3 years ago

Hi @dkassab Thanks for the PR there are two lint errors:

A missing newline

Step #1 - "lint-tests": Checking for missing newline at end of file
Step #1 - "lint-tests": Error: No newline at end of file ./6-anthos-install/README.md

A missing required argument

Step #1 - "lint-tests": terraform_validate ./app-foundation/4-projects/shared 
Step #1 - "lint-tests": 
Step #1 - "lint-tests": Error: Missing required argument
Step #1 - "lint-tests": 
Step #1 - "lint-tests":   on infra_pipeline.tf line 44, in module "infra_pipelines":
Step #1 - "lint-tests":   44: module "infra_pipelines" {
Step #1 - "lint-tests": 
Step #1 - "lint-tests": The argument "impersonate_service_account" is required, but no definition was
Step #1 - "lint-tests": found

The missing required argument was fixed in PR https://github.com/GoogleCloudPlatform/terraform-example-foundation-app/pull/40 It should be fixed if you rebase with the current version of main

google-cla[bot] commented 3 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

rutalreja-deloitte commented 3 years ago

@googlebot I consent.

rutalreja-deloitte commented 3 years ago

Fixes in c599738f5cdeaa8ff6f3c5f8a09acb3b16233e89

Merged main and lint pass
Changes requested in @daniel-cit 's comment
Remove --cluster-labels not needed
Add missing instruction for frontend-sa.yaml
Instruction update that was merged in #49

rutalreja-deloitte commented 3 years ago

@dkassab and @daniel-cit Noticed missing instructions for

acm root-sync yaml in ReadME, line should also have GKE_PROJECT_ID instead of PROJECT_ID
cloud-sql-admin-secret yaml instruction to update this in ReadME
app-secret-front yaml instruction to update this in ReadME as well as the correct project env variable GKE most likely

P.S Also a lot of files in the 6-anthos-install/acm-repos/root-config-repo/namespaces/boa/ repo folders are entirely commented out, maybe some instructions in the README are required to inform user what role the files play and at what stage

dkassab commented 3 years ago

@dkassab and @daniel-cit Noticed missing instructions for

acm root-sync yaml in ReadME, line should also have GKE_PROJECT_ID instead of PROJECT_ID

cloud-sql-admin-secret yaml instruction to update this in ReadME

app-secret-front yaml instruction to update this in ReadME as well as the correct project env variable GKE most likely

P.S Also a lot of files in the 6-anthos-install/acm-repos/root-config-repo/namespaces/boa/ repo folders are entirely commented out, maybe some instructions in the README are required to inform user what role the files play and at what stage

I fixed acm root-sync.yaml file.
cloud-sql-admin-secret is not needed anymore, I removed it.
This secret is not needed for this phase, but will need it once we implement secret manager and CSI secret driver.

rutalreja-deloitte commented 3 years ago

Fixes #51

Exposed bastion_members in 5-infra/env/main.tf and added supporting instruction in 6-README
Added missing Readme instruction for acm-repo-sync
Commented out app-secret-front

rutalreja-deloitte commented 3 years ago

LGTM @daniel-cit and @bharathkkb please review when you get a chance

daniel-cit commented 3 years ago

the populate script is in the Bank of Anthos repo

cd ${HOME}

git clone https://github.com/GoogleCloudPlatform/bank-of-anthos.git

kubectl apply -n transactions --context ${CTX_1} -f ${HOME}/bank-of-anthos/extras/cloudsql/populate-jobs/populate-ledger-db.yaml

kubectl apply -n transactions --context ${CTX_2} -f ${HOME}/bank-of-anthos/extras/cloudsql/populate-jobs/populate-ledger-db.yaml