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

first cut at rough thoughts #7

Open
wants to merge 1 commit into
base: fine-grained-authz
Choose a base branch
from

Conversation

deads2k
Copy link

@deads2k deads2k commented Oct 19, 2022

very quick, 90% of my thoughts

described, but the author embeds another type and therefore cannot exclude particular
fields for fields that already have separate permissions.
Examples of this would be pod.spec.nodeName or more commonly metadata.labels.
3. Cluster-admin or other extension excluding values from shipped schema.
Copy link
Owner

@lavalamp lavalamp Oct 20, 2022

Choose a reason for hiding this comment

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

I think the existing design has made a mistake by proposing schema tags for both the permission description AND the "exclusive" bit. I think there is reason for admins to want to change the exclusive bit, but not the permission <=> field assignment (that would mess up cross-cluster compatibility / understandability). I think this is easy-ish to fix, although we'll have to make an object to store the mapping, sigh.

The existing design also makes a mistake by using the word "exclude" to mean "permission X is excluded from the general permission", that's not what people expect.

I think you're asking for real-exclude ("no matter what is implied by other permissions, you may only modify label X if you have permission X"), or at least a permission system making it possible to implement a real-exclude permission.

Copy link
Owner

Choose a reason for hiding this comment

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

I've pushed a revision that uses the word "uncovered" instead of "exclusive" for the concept currently in the design so now at least we can talk about the two concepts separately.

Copy link
Owner

Choose a reason for hiding this comment

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

I think the easiest way to get what you want is this:

  1. move the "uncovered" marker out of the schema and into a runtime-configurable place.
  2. make "uncovered" mean not just the regular permissions, but also any granular permissions that ordinarily would include it. (i.e., UPDATE isn't good enough,granular:metadata isn't good enough, you must have granular:label(pod-security.kubernetes.io) to write that label)

I think this is enough to let a zealous admin make a field exclusive to users having a particular granular permission.

I have a step three if you think this isn't good enough.

Copy link
Owner

Choose a reason for hiding this comment

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

I finally found time to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants