canada-ca / cloud-guardrails-gcp

Recommended configuration guidance for Google Cloud Platform / Conseils de configuration recommandés pour Platforme infonuagique de Google
Other
22 stars 14 forks source link

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

Closed jacyang2010 closed 1 year ago

jacyang2010 commented 1 year ago

This PR should resolve the following issues: #13

Changes:

Test Result:

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
fmichaelobrien commented 1 year ago

Fixes https://github.com/canada-ca/cloud-guardrails-gcp/issues/13 Not #11

fmichaelobrien commented 1 year ago

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 commented 1 year ago

Code change reviewed OK

LGTM

jacyang2010 commented 1 year ago

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 commented 1 year ago

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

fmichaelobrien commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Example https://github.com/GoogleCloudPlatform/pubsec-declarative-toolkit/pull/317

For this pr

LGTM Approved Ready to merge

fmichaelobrien commented 1 year ago

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

fmichaelobrien commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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

fmichaelobrien commented 1 year ago

Thank you @ptd-tbs for the merge - appreciated