open-policy-agent / gatekeeper

🐊 Gatekeeper - Policy Controller for Kubernetes
https://open-policy-agent.github.io/gatekeeper/
Apache License 2.0
3.68k stars 754 forks source link

Add a runbook link to constraints #745

Open snuggie12 opened 4 years ago

snuggie12 commented 4 years ago

Describe the solution you'd like I'm curious what everyone thinks about adding an optional field to constraints that is a URL. The purpose of the URL is a document explaining why a constraint exists and how a user can come into compliance with the constraint (obviously they could put whatever they want into the link, but just giving my usage.)

Anything else you would like to add: I also pondered about adding the individual fields themselves, but I think it's more useful to let people decide. I guess in that spirit the field could be called "Additional Info" and leave it up to them to put a link or some other text.

Environment:

maxsmythe commented 4 years ago

I like the idea of it.

I'm wondering whether it best lives as an annotation or as an additional field. IMO to justify it being an additional field, we should have some way that it affects the operation of Gatekeeper itself. Would we surface it to users somehow as part of the violation message? Do we want this field to be dynamic and change the link depending on how the constraint is violated? Is it purely for admins as they decide whether to keep an existing constraint from years ago?

I think knowing the potential audiences for this feature might inform features we may want it to have.

One thing to note is that the Rego of a constraint template can return a details field in addition to a msg field. This may be a route for customizing some of this information.

snuggie12 commented 4 years ago

I think the audience varies from company to company. For myself, we'll have non-admins reading them.

Take the rest of this with a grain of salt because I'm only 2 days new to OPA (very much love it already.) Currently I have a parameter called "link" and I do indeed bubble it up as the first part of the message (this is because I see some text getting cut off in the constraint's status.violations (perhaps details always shows up or perhaps there is a better spot to view violations?)

Likely looking too far ahead, why I thought it could be part of the resource itself was because I also wanted a way to potentially audit the constraints themselves. I couldn't rationalize a way to make a policy monitoring for links with it just being a parameter and thought it might work better if it was an optional switch either on the pod or the webhook configuration.

The reason I wanted to bring it up though was to see if people felt like it would add to the overall product/ecosystem (i.e. other compliance frameworks like PCI have controls stating reasoning and how to comply.)

Should they be separate CRDs all together? On the one hand etcd seems like a bizarre space to store that much text, but on the other hand some config maps are huge?

If there's an easy way to implement I'll be happy with that. I hadn't thought about an annotation. That could work though I think you might not see the link if you are creating a new resource in violation of a policy?

maxsmythe commented 4 years ago

Ensuring constraints have some kind of documentation around their purpose and whether/which compliance regimes they serve is definitely interesting. I think having a common format for aggregating such data could be very helpful for a lot of people with compliance concerns. Careful thought would need to be put in to make sure that we have enough reporting granularity and are aggregation-friendly enough to deal with multiple different compliance regimes and implementations of those regimes.

Because constraints themselves are units of enforcement, I think it makes sense to embed which compliance requirements they help meet directly in the constraint.

Unfortunately we currently have no universal way to customize the output of a constraint, though this is a strong argument for doing something like that. Because this would involve changing the behavior of the constraint itself, it moves the data outside the scope of annotations, which are K8s way of integrating side-functionality into existing resources.

The main reason why you are seeing truncating of violation messages is that Kubernetes only allows for a finite space per-resource. Because the number of audit violations and the size of each violation's contents are theoretically unbounded, we put hard caps to both fit within the bounds of K8s space limitations and to limit memory usage for processes like audit.

Currently the best place to view detailed audit violations is in the audit pod's logs, where violations are output in standardized JSON. Aggregate violation information is exposed via Prometheus metrics. There is also an alpha effort to expose violation information via events. I expect the behavior of the events functionality to stabilize overtime once we know how event reporting behaves at scale.

ritazh commented 4 years ago

document explaining why a constraint exists and how a user can come into compliance with the constraint

+1

I think this is especially useful for the returned deny message as part of the review response. But it would most certainly be cutoff as part of the constraint's status.violations due to the object size limitation mentioned above. As we test the emit events feature, we should keep this in mind to see if it's possible to add this.

jpreese commented 4 years ago

@snuggie12 we're just now starting to look into how best to approach runbooks with konstraint. While it's easy enough to add a URL in the header comments, which will show up in the documentation, we'd like to see it included in the violation itself.

As @maxsmythe mentioned, one avenue we considered was just adding an annotation during the generation process, but the user would still have to inspect the constraint itself. We'd like to be able to present the link to the user in the violation message.

details seems to be the best candidate for this, but we're not quite sure how that gets to the user yet. While primative, another idea we tossed about was during the generation process, just edit the msg value to include runbook: url.com/here.

ritazh commented 4 years ago

details seems to be the best candidate for this, but we're not quite sure how that gets to the user yet.

+1 We are currently not returning the details data in the admission response, but we could easily do this. https://github.com/open-policy-agent/gatekeeper/blob/master/pkg/webhook/policy.go#L278

jpreese commented 4 years ago

That would be huge! If rendering the details makes sense to the Gatekeeper team, I'd be more than happy to open a PR.

maxsmythe commented 4 years ago

What design did you have in mind for surfacing the details returned by a violation? This strikes me as having the potential of extending the current API for constraints/templates, so it may be worth looking at the high-level plan to be sure it's flexible enough to fit other potential future use cases without breaking backwards compatibility.