infra/gcp: enable GCS access logs for staging project GCS and GCR#2231
infra/gcp: enable GCS access logs for staging project GCS and GCR#2231k8s-ci-robot merged 3 commits intokubernetes:mainfrom
Conversation
|
Hi @bernokl. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
spiffxp
left a comment
There was a problem hiding this comment.
Thanks for starting on this! I added suggestions on how to implement
spiffxp
left a comment
There was a problem hiding this comment.
Getting there, more suggestions
|
/ok-to-test |
spiffxp
left a comment
There was a problem hiding this comment.
This should hopefully be the last round
infra/gcp/lib_gcs.sh
Outdated
| # $1: The GCS bucket | ||
| function ensure_gcs_bucket_logging() { | ||
| if [ $# != 2 ] || [ -z "$1" ]; then | ||
| echo "ensure_gcs_bucket_logging(name) requires 1 argument" >&2 | ||
| return 1 | ||
| fi | ||
| local bucket="$1" |
There was a problem hiding this comment.
For parity with other functions in this library, give an example, and stick with bucket vs. bucket name
| # $1: The GCS bucket | |
| function ensure_gcs_bucket_logging() { | |
| if [ $# != 2 ] || [ -z "$1" ]; then | |
| echo "ensure_gcs_bucket_logging(name) requires 1 argument" >&2 | |
| return 1 | |
| fi | |
| local bucket="$1" | |
| # $1: The GCS bucket (e.g. gs://k8s-infra-foo) | |
| function ensure_gcs_bucket_logging() { | |
| if [ $# != 2 ] || [ -z "$1" ]; then | |
| echo "ensure_gcs_bucket_logging(bucket) requires 1 argument" >&2 | |
| return 1 | |
| fi | |
| local bucket="$1" |
There was a problem hiding this comment.
whoops, one more... it should be [ $# != 1 ]
35cccf5 to
b8f1991
Compare
spiffxp
left a comment
There was a problem hiding this comment.
Please address shellcheck failures that are causing the verify job to fail
|
Since we're getting close, I would also prefer to see this rebased to get rid of the fixup commits, and into commits relevant to the appropriate files
|
b8f1991 to
ad7b145
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bernokl, spiffxp The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Running into a few issues while trying to deploy these changes, opened #2283 with the necessary fixes to deploy |
|
Running |
|
Finished deploying |
Addresses:
#904 (comment)
Intent:
Research:
https://github.com/ii/org/blob/main/research/k8sio-ensure-staging-pii-gcs.org