Skip to content

Refresh infra scripts to address audit followups, reduce output noise#1859

Merged
k8s-ci-robot merged 16 commits intokubernetes:mainfrom
spiffxp:misc-infra-script-fixes
Apr 8, 2021
Merged

Refresh infra scripts to address audit followups, reduce output noise#1859
k8s-ci-robot merged 16 commits intokubernetes:mainfrom
spiffxp:misc-infra-script-fixes

Conversation

@spiffxp
Copy link
Copy Markdown
Contributor

@spiffxp spiffxp commented Mar 31, 2021

I've run these a few times locally and the results are showing up in audit PRs, so I should probably share what I'm working on.

Will update this description to link to issues these fixes resolve once I've taken care of the WIP commits

Should allow us to address #1675

  • Running K8S_INFRA_ENSURE_ONLY_SERVICES_WILL_FORCE_DISABLE=true ./infra/gcp/ensure-staging-storage.sh should disable all of the unexpected services. I plan on doing a first pass without that set to get a log of which projects would have which services disabled first.

Fixes #299

  • Any new projects going forward will have the user:foo roles/owner binding removed immediately after the project is created. I plan on running ./infra/gcp/*.sh to ensure the roles/owner binding is removed for all existing projects.

See individual commits for details

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 31, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. wg/k8s-infra approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 31, 2021
@spiffxp spiffxp force-pushed the misc-infra-script-fixes branch 9 times, most recently from c224676 to 22e6015 Compare April 6, 2021 18:53
@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

I ran ./infra/gcp/prow/ensure-e2e-projects.sh as of 22e6015

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

I ran K8S_INFRA_ENSURE_ONLY_SERVICES_WILL_FORCE_DISABLE=true ./infra/gcp/ensure-staging-storage.sh e2e-test-images as of 22e6015 to confirm services were disabled (I know this is at least one project I accidentally enabled a bunch of services on while navigating through GCP's UI as an org admin)

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

I ran ./infra/gcp/prow/ensure-main-project.sh as of 22e6015

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

Ran ./infra/gcp/ensure-staging-storage.sh, script terminated at sig-storage

Configuring staging: sig-storage
  Ensuring project exists: k8s-staging-sig-storage
  ERROR: (gcloud.projects.remove-iam-policy-binding) Resource in projects [k8s-staging-sig-storage:setIamPolicy] is the subject of a conflict: There were concurrent policy changes. Please retry the whole read-modify-write with exponential backoff.

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

Continued with ./infra/gcp/ensure-staging-storage.sh sig-storage sp-operator storage-migrator txtdirect

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

Going to disable extraneous services in staging projects as listed in #1675 (comment)

export PLAN_URL=https://gist.githubusercontent.com/spiffxp/1cbf779d7dc1c025a445b91909f55bf7/raw/49971cd6699c4dace4f47fcba8c068c212e0ad2e/k8s-staging-service-disable-plan.yaml 
K8S_INFRA_ENSURE_ONLY_SERVICES_WILL_FORCE_DISABLE=true \
  ./ensure-staging-storage.sh \
  $(curl -s "${PLAN_URL}" \
    | yq -r 'keys | .[]' \
    | sed -e 's/k8s-staging-//g')

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 6, 2021

Extraneous services in k8s-staging-* projects have been disabled, see https://gist.github.com/spiffxp/1cbf779d7dc1c025a445b91909f55bf7 for log output

@spiffxp spiffxp force-pushed the misc-infra-script-fixes branch from e8c7436 to 072ef48 Compare April 7, 2021 05:31
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 7, 2021
spiffxp added 4 commits April 7, 2021 01:46
specifically

- use ensure_only_services
- use ensure_secret_role_binding
- use TMPDIR
specifically

- use ensure_only_services
- use lib_iam funcs
specifically

- use ensure_only_services
- use lib_iam and lib_gcs funcs for role bindings
- use TMPDIR instead of non-portable mktemp -p
specifically

- only create predefined projects (same bug ensure-staging-projects had)
- fix some shellcheck nits
- use ensure_only_services
- add support for resetting ssh-keys (since it's not clear if we want to
  do this by default, gate behind setting the obnoxiously long env var
  K8S_INFRA_ENSURE_E2E_PROJECTS_RESETS_SSH_KEYS=true
- reduce output noise by using lib_iam funcs
@spiffxp spiffxp force-pushed the misc-infra-script-fixes branch from 072ef48 to 28a3c3b Compare April 7, 2021 05:49
spiffxp added 4 commits April 7, 2021 01:50
specifically

- use ensure_only_services to clean up manually enabled services
- use lib_iam funcs for less noise
- move k8s-staging-releng-test special case into its own func
  to prep making this non-special-case for k8s-infra-prow-build-trusted
- sort projects
@spiffxp spiffxp force-pushed the misc-infra-script-fixes branch from 28a3c3b to 5357b89 Compare April 7, 2021 05:51
@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 7, 2021

As of 5357b89 I ran the following:

  • ./infra/gcp/ensure-conformance-storage.sh
  • ./infra/gcp/ensure-gsuite.sh
  • ./infra/gcp/ensure-organization.sh (using roles refreshed via ./infra/gcp/roles/generate-role-yaml.sh)
  • ./infra/gcp/ensure-release-projects.sh
  • ./infra/gcp/ensure-releng.sh

Earlier (see comments above), I ran:

  • ./infra/gcp/prow/ensure-e2e-projects.sh
  • ./infra/gcp/ensure-main-project.sh
  • ./infra/gcp/ensure-staging-storage.sh (with K8S_INFRA_ENSURE_ONLY_SERVICES_WILL_FORCE_DISABLE=true for some after manual review)

I didn't touch them as part of this PR, but for grins (and mostly to close #299), I also ran:

  • ./infra/gcp/ensure-prod-storage.sh
  • ./infra/gcp/ensure-static-ips.sh

@spiffxp spiffxp changed the title [wip] misc infra script mixes Refresh infra scripts to address audit followups, reduce output noise Apr 7, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2021
@ameukam
Copy link
Copy Markdown
Member

ameukam commented Apr 7, 2021

/lgtm
/hold
Remove at your convenience.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 7, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 7, 2021
@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 8, 2021

Taking one last look at latest audit PR results before removing hold #1874

@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Apr 8, 2021

/hold cancel
/assign @thockin
if you want to review post-merge

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2021
@k8s-ci-robot k8s-ci-robot merged commit 3439722 into kubernetes:main Apr 8, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Apr 8, 2021
@spiffxp spiffxp deleted the misc-infra-script-fixes branch April 8, 2021 19:07
@spiffxp
Copy link
Copy Markdown
Contributor Author

spiffxp commented Jun 11, 2021

Related to refactoring infra/gcp, ref: #516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infra turnup: Whoever creates the project is listed as Owner

4 participants