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

Fixed the rego parse error and hardcoded resource names in rego policy (ready to merge) #14

Conversation

jacyang2010
Copy link
Contributor

@jacyang2010 jacyang2010 commented Apr 17, 2023

This PR should resolve the following issues: #13

Changes:

  • Refactroed the rego policy number 11
  • Upgraded the conftest version to latest

Test Result:

  • Verified that the rego parse error for the number 11 rego policy is gone from cloud build log.
  • Verified that the number 11 rego policy validation passed from cloud build log.

Please see the test result evidence from the cloud build logs below.

starting build "1148baff-48fe-41af-a976-3bc5fcfe15f6"

FETCHSOURCE
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint: 
hint: 	git config --global init.defaultBranch <name>
hint: 
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint: 
hint: 	git branch -m <name>
Initialized empty Git repository in /workspace/.git/
From https://source.developers.google.com/p/lzpe-js08-guardrailsjs08/r/LzPeCLD-guardrails-policies-csr
 * branch            4aa15a70e4eab80f7adac9b8d7de2160a676309f -> FETCH_HEAD
HEAD is now at 4aa15a7 Patched at 2023-04-18.1804
BUILD
Starting Step #0
Step #0: Already have image (with digest): gcr.io/cloud-builders/gcloud
Step #0: Copying gs://lzpe565977066779assetsguardrailsjs08/organizations/565977066779.json...
Step #0: / [0 files][    0.0 B/  3.2 MiB]                                                
/ [1 files][  3.2 MiB/  3.2 MiB]                                                
Step #0: Operation completed over 1 objects/3.2 MiB.                                      
Finished Step #0
Starting Step #1
Step #1: Already have image (with digest): gcr.io/cloud-builders/docker
Finished Step #1
Starting Step #2
Step #2: Already have image (with digest): gcr.io/cloud-builders/docker
Step #2: Unable to find image 'northamerica-northeast1-docker.pkg.dev/lzpe-js08-guardrailsjs08/lzpecld-guardrails-af-registry-afr/lzpeccr-guardrails-policies-cntr:latest' locally
Step #2: latest: Pulling from lzpe-js08-guardrailsjs08/lzpecld-guardrails-af-registry-afr/lzpeccr-guardrails-policies-cntr
Step #2: 26c5c85e47da: Already exists
Step #2: f54c0c4d962e: Pulling fs layer
Step #2: a6e04e46b0b0: Pulling fs layer
Step #2: a0c34b0db63b: Pulling fs layer
Step #2: f54c0c4d962e: Verifying Checksum
Step #2: f54c0c4d962e: Download complete
Step #2: a6e04e46b0b0: Verifying Checksum
Step #2: a6e04e46b0b0: Download complete
Step #2: f54c0c4d962e: Pull complete
Step #2: a6e04e46b0b0: Pull complete
Step #2: a0c34b0db63b: Verifying Checksum
Step #2: a0c34b0db63b: Download complete
Step #2: a0c34b0db63b: Pull complete
Step #2: Digest: sha256:c786cddd81554911dcae89e445d6ecbf5a3ff1236f340ac6789315e6514eaf68
Step #2: Status: Downloaded newer image for northamerica-northeast1-docker.pkg.dev/lzpe-js08-guardrailsjs08/lzpecld-guardrails-af-registry-afr/lzpeccr-guardrails-policies-cntr:latest
Step #2: Checking ./assets/asset_inventory.json
Finished Step #2
Starting Step #3
Step #3: Already have image (with digest): gcr.io/cloud-builders/docker
Step #3: ./assets/asset_inventory.json
Step #3: +---------+------+-----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
Step #3: | RESULT  | FILE | NAMESPACE |                                                                                                        MESSAGE                                                                                                        |
Step #3: +---------+------+-----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
Step #3: | success | -    | main      | SUCCESS                                                                                                                                                                                                               |
Step #3: | success | -    | main      | SUCCESS                                                                                                                                                                                                               |
Step #3: | success | -    | main      | SUCCESS                                                                                                                                                                                                               |
Step #3: | failure | -    | main      | Guardrail # 5: Resource containerregistry.googleapis.com/Image                                                                                                                                                        |
Step #3: |         |      |           | ('//containerregistry.googleapis.com/us.gcr.io/gdse-ja-secguardrails/gcf/northamerica-northeast1/12517d9e-8c83-4602-b943-1fafb816343b/cache@sha256:ad9f0ccb7b82f76e4d4729595cc07dbcc35547c236a6a496746b2007293bfd95') |
Step #3: |         |      |           | is located in 'us' when it is required to be in '["northamerica-northeast1", "global"]'                                                                                                                               |
Step #3: | failure | -    | main      | Guardrail # 5: Resource containerregistry.googleapis.com/Image                                                                                                                                                        |
Step #3: |         |      |           | ('//containerregistry.googleapis.com/us.gcr.io/gdse-ja-secguardrails/gcf/northamerica-northeast1/12517d9e-8c83-4602-b943-1fafb816343b@sha256:85718805cf97613355494b8bd534418f0b94ace9b923e051dc7fbdd2866b8124')       |
Step #3: |         |      |           | is located in 'us' when it is required to be in '["northamerica-northeast1", "global"]'                                                                                                                               |
Step #3: | failure | -    | main      | Guardrail # 5: Resource containerregistry.googleapis.com/Image                                                                                                                                                        |
Step #3: |         |      |           | ('//containerregistry.googleapis.com/us.gcr.io/gdse-ja-secguardrails/gcf/northamerica-northeast1/12517d9e-8c83-4602-b943-1fafb816343b@sha256:a8689a312bf2db549ecc62c683bff3bf7b126317599ff9a8d733c72f9236f049')       |
Step #3: |         |      |           | is located in 'us' when it is required to be in '["northamerica-northeast1", "global"]'                                                                                                                               |
Step #3: | failure | -    | main      | Guardrail # 5: Resource containerregistry.googleapis.com/Image                                                                                                                                                        |
Step #3: |         |      |           | ('//containerregistry.googleapis.com/us.gcr.io/gdse-ja-secguardrails/gcf/northamerica-northeast1/e2d06603-d425-45f6-931d-65ad88761d22/cache@sha256:88b0e417217136757eea27578f1222cef40304cc551ba2abbfcfcb99c0ec362f') |
Step #3: |         |      |           | is located in 'us' when it is required to be in '["northamerica-northeast1", "global"]'                                                                                                                               |
Step #3: | failure | -    | main      | Guardrail # 5: Resource containerregistry.googleapis.com/Image                                                                                                                                                        |
Step #3: |         |      |           | ('//containerregistry.googleapis.com/us.gcr.io/gdse-ja-secguardrails/gcf/northamerica-northeast1/e2d06603-d425-45f6-931d-65ad88761d22@sha256:86daf3b023d0cb47b3d970cd09f415bd4d9bb69c2f70f224838772d4daaf1314')       |
Step #3: |         |      |           | is located in 'us' when it is required to be in '["northamerica-northeast1", "global"]'                                                                                                                               |
Step #3: +---------+------+-----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
Step #3: 
Finished Step #3
Starting Step #4
Step #4: Already have image (with digest): gcr.io/cloud-builders/docker
Finished Step #4
Starting Step #5
Step #5: Already have image (with digest): gcr.io/cloud-builders/gcloud
Step #5: Copying file:///assets/565977066779.json [Content-Type=application/json]...
Step #5: / [0 files][    0.0 B/  5.3 KiB]                                                
/ [1 files][  5.3 KiB/  5.3 KiB]                                                
Step #5: Operation completed over 1 objects/5.3 KiB.                                      
Finished Step #5
PUSH
DONE

@jacyang2010 jacyang2010 changed the title GCT-43 Fixed the confest version and refactored rego policy #11 Guardrails failure Issues and hardcoded resource names in rego policy #13 Apr 17, 2023
@jacyang2010 jacyang2010 changed the title Guardrails failure Issues and hardcoded resource names in rego policy #13 Guardrails failure Issues and hardcoded resource names in rego policy Apr 17, 2023
@jacyang2010 jacyang2010 changed the title Guardrails failure Issues and hardcoded resource names in rego policy Fixed guardrails failure Issues and hardcoded resource names in rego policy Apr 17, 2023
@jacyang2010 jacyang2010 changed the title Fixed guardrails failure Issues and hardcoded resource names in rego policy Fixed guardrails failure issues and hardcoded resource names in rego policy Apr 17, 2023
@fmichaelobrien
Copy link
Contributor

Fixes #13
Not #11

@fmichaelobrien
Copy link
Contributor

LGTM
Can you put some text around the issue that occurred and some test results after the fix

We can then merge it

The procedure for PRs for canada-ca is to get 1+ approval reviews and then update the title with something like "ready to merge" - see PRs already committed -timeline is 2 to 14 days, use your fork repo for dev work until the pr is in

@fmichaelobrien
Copy link
Contributor

Code change reviewed OK

LGTM

@jacyang2010
Copy link
Contributor Author

LGTM Can you put some text around the issue that occurred and some test results after the fix

We can then merge it

The procedure for PRs for canada-ca is to get 1+ approval reviews and then update the title with something like "ready to merge" - see PRs already committed -timeline is 2 to 14 days, use your fork repo for dev work until the pr is in

OK

@jacyang2010 jacyang2010 changed the title Fixed guardrails failure issues and hardcoded resource names in rego policy #13 - Guardrails failure issues and hardcoded resource names in rego policy Apr 18, 2023
@jacyang2010 jacyang2010 changed the title #13 - Guardrails failure issues and hardcoded resource names in rego policy Fixed the rego parse error and hardcoded resource names in rego policy Apr 18, 2023
@jacyang2010
Copy link
Contributor Author

I have no permission to invite people to review this PR. @fmichaelobrien

@fmichaelobrien
Copy link
Contributor

fmichaelobrien commented Apr 18, 2023

The pbmm and pubsec repos are under the google org so access can be granted and invites to the PR can be assigned there normally. None of us are part of this CA repo so PR reviews are done adhoc by pasting a note to this PR of something like "approved".

If it is made clear through comments that 1+ other GitHub users from google traditionally and now from your org - approve the pr and optionally view or run the patch. We are good.

The org owner will merge the pr then

As it stands there is one approving review

@jacyang2010 jacyang2010 changed the title Fixed the rego parse error and hardcoded resource names in rego policy Fixed the rego parse error and hardcoded resource names in rego policy (ready to merge) Apr 18, 2023
@jacyang2010
Copy link
Contributor Author

jacyang2010 commented Apr 18, 2023

Code change reviewed OK

LGTM

I will treat this message as an approval varient even though the expected keyword "approved" is not mentioned. Please let me know if I was wrong. @fmichaelobrien

@fmichaelobrien
Copy link
Contributor

Yes on the pubsec repo we usually sign an approval with LGTM

Example GoogleCloudPlatform/pubsec-declarative-toolkit#317

For this pr

LGTM
Approved
Ready to merge

@fmichaelobrien
Copy link
Contributor

@ptd-tbs and team PR was reviewed and PR is ready to merge

@fmichaelobrien
Copy link
Contributor

fmichaelobrien commented May 16, 2023

@ptd-tbs the PR is ready to merge
@jacyang2010 for the time being - you can fork the repo with the PR change and use the fork until this upstream is merged
Use the existing fork that is the pr source below
https://github.com/yw-liftandshift/cloud-guardrails-gcp
from
https://github.com/canada-ca/cloud-guardrails-gcp/forks

@jacyang2010
Copy link
Contributor Author

@fmichaelobrien we are actually not blocked by this PR and we have patched the clone of this repo before pushing it to the guardrails CSR of the landing zone deployment instance. Thanks for your reminder anyways.

@fmichaelobrien
Copy link
Contributor

Sounds good, I was pinged by management about a blocker here yesterday.

@ptd-tbs ptd-tbs merged commit 52a8888 into canada-ca:main May 19, 2023
@fmichaelobrien
Copy link
Contributor

Thank you @ptd-tbs for the merge - appreciated

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.

3 participants