Automated RBAC generation and verification

nolar commented 5 years ago

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.
- [ ] https://kopf.readthedocs.io/en/stable/deployment/
Tests:
- [ ] CLI tests.
- [ ] RBAC parsing tests.
- [ ] RBAC verification tests.

rosscdh commented 5 years ago

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

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

rosscdh commented 5 years ago

also the events access was missing from both clusterrole and role

nolar commented 5 years ago

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?

rosscdh commented 5 years ago

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 https://github.com/zalando-incubator/kopf/pull/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 https://github.com/zalando-incubator/kopf/pull/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 https://github.com/zalando-incubator/kopf/issues/49?email_source=notifications&email_token=AADA6MTGFQL2TUOPC3PSEVLPYM4LLA5CNFSM4HJBZFB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWXM4UQ#issuecomment-497995346, or mute the thread https://github.com/notifications/unsubscribe-auth/AADA6MQI6YH7A6BN46AJXKTPYM4LLANCNFSM4HJBZFBQ .

rosscdh commented 5 years ago

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 https://github.com/zalando-incubator/kopf/pull/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 https://github.com/zalando-incubator/kopf/pull/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 https://github.com/zalando-incubator/kopf/issues/49?email_source=notifications&email_token=AADA6MTGFQL2TUOPC3PSEVLPYM4LLA5CNFSM4HJBZFB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWXM4UQ#issuecomment-497995346, or mute the thread https://github.com/notifications/unsubscribe-auth/AADA6MQI6YH7A6BN46AJXKTPYM4LLANCNFSM4HJBZFBQ .

nolar commented 5 years ago

@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.

rosscdh commented 5 years ago

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 https://github.com/zalando-incubator/kopf/issues/49?email_source=notifications&email_token=AADA6MX2MDGIAMXFIVTVKDLPYPBZJA5CNFSM4HJBZFB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWXVN4Y#issuecomment-498030323, or mute the thread https://github.com/notifications/unsubscribe-auth/AADA6MTSY36MKQJ3EAJOGITPYPBZJANCNFSM4HJBZFBQ .

nolar commented 5 years ago

@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)?

rosscdh commented 5 years ago

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

nolar commented 5 years ago

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

zalando-incubator / kopf