infra/gcp: disable containerscanning and test-infra-trusted prow access for staging and release projects #2016
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: spiffxp The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
k8s-ci-robot
added
size/XL
|
#2067 is starting to pick up the results of me running some of these scripts while developing/testing this I'm going to kick off a run of ensure-staging-storage.sh as I'm confident based on testing, but will refrain from running the rest until review. |
spiffxp
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
ameukam
reviewed
May 20, 2021
Comment on lines
+135
to
+136
| color 6 "Ensuring k8s-prow / test-infra-trusted can no longer use GCB in project: ${PROJECT}" | ||
| ensure_removed_google_prow_bindings "${PROJECT}" "${GCB_BUCKET}" |
There was a problem hiding this comment.
I'm not sure but this part is applied once. Why do it ever time the script is used ?
There was a problem hiding this comment.
The idea is to remove this in a follow-up PR. I was starting to get lazy by this point so I didn't go as far as making a "bindings to remove" array like I did in ensure-organizations.
|
#2067 now reflects all changes for release and staging projects |
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
size/XXL
Describe the reason includes are sourced in the way they are Pull more constants into lib.sh and organize them by "purpose" and then alphabetically within. The following were added to lib, or scripts were modified to use these: - RELEASE_ADMINS, RELEASE_WRITERS, RELEASE_VIEWERS - PROW_GOOGLE_TRUSTED_SERVICE_ACCOUNT (formerly PROW_SVCACCT) - PROW_BUILD_SERVICE_ACCOUNT - PROW_TRUSTED_BUILD_CLUSTER_PROJECTS - PROW_UNTRUSTED_BUILD_CLUSTER_PROJECTS Shellcheck complains that some of these aren't used within lib.sh and should be exported if they're used elsewhere. Going to intentially hold off on that for now, it may turn out not all of these need to be "global", or that there's a better place to store these.
This was broken by a missing PROJECT_ID variable (probably by me in a previous attempt to refactor into functions) Fixed that, and did some other misc fixes: - move services to a readonly constant up top - use `project` instead of `project_id` - fix shellcheck nits - avoid funcname hardcode on args checks
Planning to overhaul ensure_services_only, so move service-related functions into their own lib_service.sh Moved the following over: - enable_api - _plan_enabled_services - ensure_only_services
specifically: - write the .jq file used by ensure_only_services only once - fix shellcheck nits - enable_api now supports N services; will rename in later commit
Since the gcloud command is `gcloud services` let's drop the API nomenclature, and pluralize to reflect that N services can be enabled at once now. Add a corresponding ensure_disabled_services function with a scary warning. While following the the rename, I also refactored to use lists of services if relevant, for: - ensure-prod-storage.sh - ensure-prod-storage-gclb.sh - ensure-env-cip-auditor.sh
Following a pattern that I've used for ensure-main-project.sh and ensure-organization.sh that involves moving everything into functions or readonly constants, and then an entrypoint at the bottom. So the entrypoint is no longer a massive for loop and blocks of code, but instead ensure_staging_projects, which calls: - ensure_staging_project for each repo, which calls: - ensure_staging_gcs_bucket - ensure_staging_gcr_repo - ensure_staging_gcb - staging_special_case__k8s_staging_foo (if it exists) - ensure_release_manager_special_cases I commented functions heavily, and dropped comments if the logging statement was basically saying the same thing. Other changes: - Stop using empower_prow, prep for using service-account per-project - Change ensure_staging_gcb_builder_service_account to also grant write access to GCS and GCR directly from that service account
There are no longer any jobs that run in test-infra-trusted using the deployer serviceaccount. Revoke that serviceaccount's access to any of the k8s-staging* or k8s-release* projects
Since I still don't trust ensure_only_services to do the right thing by default, and I know we want to disable the containerscanning service, add support for explicitly disabling services.
|
Well, I did more than fix the typo. Tried to rebase changes to make a little more sense. But I'm done adding anything more to this PR for real. The summary up top is still correct. |
|
Here's what I have run from HEAD of this PR so far
|
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
late /lgtm |
This was referenced May 21, 2021
|
Related to : #516 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an attempt at making some progress on the audit followup issue to ensure we don't have random services enabled #1887
It's also more directly motivated by ensuring that containerscanning is disabled for #1963
EDIT: I'm going to cap this off before it gets too sprawling. See individual commits for details.
Brief summary:
lib_services.sh, renameensure_apitoensure_serviceslib.shensure-staging-storage.shinto functions, as has been done forensure-organization.shandensure-main-project.shensure_disabled_servicesdeployer@k8s-prowbindings from staging/release projects,test-infra-trustedno longer runs jobs hereensure_staging_gcb_builder_service_accounttoward something that will be used by default for all staging projects, to implement the "gcb account per staging project" approach described in Mitigate image-pushing jobs hitting GetRequestsPerMinutePerProject quota for prow build cluster project test-infra#20652 (comment) (which should also address cloudbuild job failed to access k8s-artifacts-csi #1544)