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

Create a prow.viewer custom org role #1061

Merged
merged 2 commits into from
Jul 24, 2020

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented Jul 24, 2020

I want to empower others to see and troubleshoot whatever resources
constraints (or lack thereof) the release-blocking jobs are running under

Members of the k8s-infra-prow-viewers@kubernetes.io get this role on
projects related to prow

The role is a composite of:

  • roles/compute.viewer
  • roles/container.viewer
  • roles/logging.viewer
  • roles/monitoring.view

It's defined by a file and will be updated anytime
infra/gcp/ensure-e2e-projects.sh is run

I've currently manually created it and applied it to the
k8s-infra-prow-build project to verify access to expected resources

Members who have this role should be able to see two custom cloud
monitoring dashboards:

I want to empower others to see and troubleshoot whatever resources
constraints (or lack thereof) the release-blocking jobs are running under

Members of the k8s-infra-prow-viewers@kubernetes.io get this role on
projects related to prow

The role is a composite of:
- roles/compute.viewer
- roles/container.viewer
- roles/logging.viewer
- roles/monitoring.view

It's defined by a file and will be updated anytime
infra/gcp/ensure-e2e-projects.sh is run

I've currently manually created it and applied it to the
k8s-infra-prow-build project to verify access to expected resources
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 24, 2020
@k8s-ci-robot k8s-ci-robot added wg/k8s-infra approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 24, 2020
@spiffxp
Copy link
Member Author

spiffxp commented Jul 24, 2020

/cc @BenTheElder @liggitt
In case you have suggestions on what else people should be able to see

@BenTheElder
Copy link
Member

cluster view maybe?

fi
}

function custom_org_role_name() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do with comments

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all LGTM. I have waffled in my head about using custom roles more. It would clearly simplify a lot of logic, but the fact that you have to expand premade roles like compute.viewer into their constituent parts, means that any further updates to that role will not automatically propagate to this role.

It's probably fine in practice, it's just distasteful to me.

@thockin
Copy link
Member

thockin commented Jul 24, 2020

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spiffxp, thockin

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 2ca6276 into kubernetes:master Jul 24, 2020
@dims
Copy link
Member

dims commented Jul 24, 2020

@spiffxp i can confirm that i can see both dashboard! (had to drop the authuser=1 from the url though)

@spiffxp spiffxp deleted the empower-prow-viewers branch July 24, 2020 23:55
@spiffxp
Copy link
Member Author

spiffxp commented Jul 24, 2020

you have to expand premade roles like compute.viewer into their constituent parts

I wasn't thrilled by this either. I didn't script creation of this role, which would be the next logical step. Could periodically re-run that script to re-generate the role, so updates would propagate, but on a schedule of our own making.

I'll address the lack of comments in a followup, thanks.

@spiffxp
Copy link
Member Author

spiffxp commented Jul 25, 2020

Ran ./infra/gcp/ensure-e2e-projects.sh

@spiffxp spiffxp added the area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters label Aug 25, 2020
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants