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

Automated RBAC generation and verification #49

Open
11 tasks
kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Open
11 tasks

Automated RBAC generation and verification #49

kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Labels
archive enhancement New feature or request

Comments

@kopf-archiver
Copy link

kopf-archiver bot commented Aug 18, 2020

An issue by nolar at 2019-04-29 09:44:00+00:00
Original URL: zalando-incubator/kopf#49
 

Background

With kopf>=0.9, the operators fail to start in the clusters with RBAC configured according to the docs. Introduced by #38, where GET is used on a specific peering object (not just on a list).

The deployment docs were not updated to reflect that. And, even if updated, that would lead to these incidents anyway, as the RBAC yaml file is not auto-validated and not auto-updated in our case, so we would not notice the change.

Suggestion: RBAC verification

Kopf should allow to verify if the RBAC yaml file matches the framework's and operator's expectations, and explain what is missing:

kopf rbac verify script1.py script2.py -f rbac.yaml

This verification step could be optionally used either in CI/CD testing stage, or in the docker build stage, and to fail the build if the source-code RBAC yaml file lacks some necessary permissions.

If no -f option is specified (OR: if --cluster is explicitly specified — TDB), then verify against the real currently authenticated cluster:

kopf rbac verify script1.py script2.py --cluster

The output should explain what is missing:

# Kopf's internals:
KopfPeering get permission: ❌absent
KopfPeering list permission: ✅present
KopfPeering watch permission: ✅present
KopfPeering patch permission: ✅present

# Used at script1.py::create_fn():
KopfExample list permission: ✅present
KopfExample watch permission: ✅present
KopfExample patch permission: ✅present

Some permissions are missing. The operator will fail to work.
Read more at https://kopf.readthedocs.io/en/stable/deployment/
Or use `kopf rbac generate --help`

Exit status should be 0 (all is okay) or 1 (something is missing), so that it could be used in CI/CD.

Suggestion: RBAC generation

Since Kopf would already contain the RBAC parsing & analysis logic, it is also fine to generate the RBAC yaml files from the codebase of the operator — based on which resources/events/causes are registered for handling (same CLI semantics as in kopf run: -m module or file.py).

kopf rbac generate script1.py script2.py > rbac.yaml
kubectl apply -f rbac.yaml

Extra: children objects introspection

As a challenge, some introspection might be needed into the internals of the handlers on which children objects they manipulate from the handlers (e.g. pod creation) — this must also be part of the RBAC docs. Or an additional decorator to declare these objects on the handler functions.

Acceptance Criteria

  • Implementation:
    • RBAC generation to stdout.
    • RBAC generation to file (-o, --output).
    • RBAC verification of stdin.
    • RBAC verification of file (-f, --file).
    • RBAC verification of cluster (--cluster).
    • Explanation of present/absent permissions.
    • Exit status on verification.
  • Documentation.
  • Tests:
    • CLI tests.
    • RBAC parsing tests.
    • RBAC verification tests.

Commented by rosscdh at 2019-05-31 13:44:02+00:00
 

docs from readthe docs seem to be missing for both role and clusterrole

  - apiGroups: [apiextensions.k8s.io]
    resources: [customresourcedefinitions]
    verbs: [list, watch, patch, get]


Commented by rosscdh at 2019-05-31 21:12:07+00:00
 

also the events access was missing from both clusterrole and role


Commented by nolar at 2019-06-02 03:23:32+00:00
 

The events' RBAC is fixed in #89 — the events API was changed from v1beta1 to core v1, but the docs were not in sync with that.

For the customresourcedefinitions — thanks for pointing out. Fixed in #95 — both the docs, and the code for the case when cluster-scoped RBAC is not possible.

rosscdh Speaking of which, what do you mean by "missing from both clusterroles and role"? Events are namespaced, aren't they? What is the purpose of the cluster-scoped events RBAC? Or is it only for the purpose of creating the events globally, without the individual per-namespace role?


Commented by rosscdh at 2019-06-02 05:45:20+00:00
 

That's a damned good point.. I'll review the scoping as you pointed out.. I
think at the point that I got it working I was keysmashing.. so most
possible it needs a cleanup...
https://github.com/rosscdh/crd-route53/blob/master/kustomize/base/rbac.yml

Mate I must thank you for your efforts on this project itd made getting
into operators several shades easier thanks to your efforts.. I owe you at
least a beer.

On Sun, 2 Jun 2019, 05:23 Sergey Vasilyev, notifications@github.com wrote:

The events' RBAC is fixed in #89
#89 — the events API was
changed from v1beta1 to core v1, but the docs were not in sync with that.

For the customresourcedefinitions — thanks for pointing out. Fixed in #95
#95 — both the docs, and
the code for the case when cluster-scoped RBAC is not possible.

rosscdh https://github.com/rosscdh Speaking of which, what do you mean
by "missing from both clusterroles and role"? Events are namespaced,
aren't they? What is the purpose of the cluster-scoped events RBAC? Or is
it only for the purpose of creating the events globally, without the
individual per-namespace role?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#49,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADA6MQI6YH7A6BN46AJXKTPYM4LLANCNFSM4HJBZFBQ
.


Commented by rosscdh at 2019-06-02 05:50:38+00:00
 

P.S. I was thinking of using something like marshmallow to validate the
spec. Is this a strategy you would endorse? or do you have something else
in mind?

On Sun, 2 Jun 2019, 07:52 Ross, sendrossemail@gmail.com wrote:

That's a damned good point.. I'll review the scoping as you pointed out..
I think at the point that I got it working I was keysmashing.. so most
possible it needs a cleanup...
https://github.com/rosscdh/crd-route53/blob/master/kustomize/base/rbac.yml

Mate I must thank you for your efforts on this project itd made getting
into operators several shades easier thanks to your efforts.. I owe you at
least a beer.

On Sun, 2 Jun 2019, 05:23 Sergey Vasilyev, notifications@github.com
wrote:

The events' RBAC is fixed in #89
#89 — the events API was
changed from v1beta1 to core v1, but the docs were not in sync with that.

For the customresourcedefinitions — thanks for pointing out. Fixed in #95
#95 — both the docs, and
the code for the case when cluster-scoped RBAC is not possible.

rosscdh https://github.com/rosscdh Speaking of which, what do you
mean by "missing from both clusterroles and role"? Events are
namespaced, aren't they? What is the purpose of the cluster-scoped events
RBAC? Or is it only for the purpose of creating the events globally,
without the individual per-namespace role?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#49,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADA6MQI6YH7A6BN46AJXKTPYM4LLANCNFSM4HJBZFBQ
.


Commented by nolar at 2019-06-02 13:16:02+00:00
 

rosscdh You are welcome!

Do you mean validation for the purpose of this issue? I never used marshmallow, so I can say nothing about its usage. But I see that it is lightweight, does not bring a lot of dependencies (actually, none), which is good — we can try.

If the size or amount of dependencies will become a problem in the future, I will move all non-runtime dependencies into extras, and make them installable via pip install 'kopf[sdk]', and ensure that all such imports are optional/conditional. But that can be ignored for now.

PS: Do you work on this issue? If so, I prefer to assign it to you, so that nobody else starts working on it in parallel.


Commented by rosscdh at 2019-06-02 18:38:01+00:00
 

I was thinking something like either another decorator or appending a
spec_validator to the current decorator
which if present would then validate just pre to being injected into the
create|delete|etc|_fn(spec)
tho problems cloud arise if there are version changes to an existing spec
whos schema then gets updated.. maybe spec versioning?
needs a bit of thought.

Maybe we make another ticket out of it?

On Sun, Jun 2, 2019 at 3:16 PM Sergey Vasilyev notifications@github.com
wrote:

rosscdh https://github.com/rosscdh You are welcome!

Do you mean validation for the purpose of this issue? I never used
marshmallow, so I can say nothing about its usage. But I see that it is
lightweight, does not bring a lot of dependencies (actually, none), which
is good — we can try.

If the size or amount of dependencies will become a problem in the future,
I will move all non-runtime dependencies into extras, and make them
installable via pip install 'kopf[sdk]', and ensure that all such imports
are optional/conditional. But that can be ignored for now.

PS: Do you work on this issue? If so, I prefer to assign it to you, so
that nobody else starts working on it in parallel.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#49,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADA6MTSY36MKQJ3EAJOGITPYPBZJANCNFSM4HJBZFBQ
.


Commented by nolar at 2019-06-04 07:36:25+00:00
 

rosscdh Hm. Sorry, I do not get how it is connected to RBAC validation/generation. Can you show an example how it could look like in the code? Or are you talking about #55 (arbitrary field validation)?


Commented by rosscdh at 2019-06-04 11:19:26+00:00
 

Sorry Nolar, I have combined a question with a ticket.

Separate question, how to ensure that the spec provided by the CRD is the expected spec.. is there a prescribed manner?

i.e in Go the specs are validated and contolled by structs, but in kopf? with dynamic python?

Regards
Ross


Commented by nolar at 2019-06-04 11:55:24+00:00
 

rosscdh Yes, that is a question for #55. I've moved this discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

0 participants