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

policy: Validate ServerAuthorization resources #8076

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Mar 16, 2022

ServerAuthorization resources are not validated by the admission
controller.

This change enables validation for ServerAuthorization resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The ServerAuthoriation validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new schemars feature of ipnet v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould ver@buoyant.io

`ServerAuthorization` resources are not validated by the admission
controller.

This change enables validation for `ServerAuthorization` resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The `ServerAuthoriation` validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new _schemars_ feature of `ipnet` v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould <ver@buoyant.io>
@olix0r olix0r requested a review from a team as a code owner March 16, 2022 00:55
Copy link
Member

@adleong adleong left a comment

Choose a reason for hiding this comment

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

Nice

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me!

Comment on lines +168 to +170
resources:
- serverauthorizations
- servers
Copy link
Member

Choose a reason for hiding this comment

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

very minor ntipick: elsewhere in the chart it looks like we use [...] syntax for YAML arrays, should we try to keep that consistent here?

Copy link
Member Author

Choose a reason for hiding this comment

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

In at least this case: no. If anything we should probably move some of the others to use explicit lists. My reasoning is that:

  1. we're going to add more soon, and this line is going to get very long.
  2. this helps minimize diffs as there are changes (lines are inserted/removed, instead of the whole line changing).

type AdmissionRequest = kube::core::admission::AdmissionRequest<DynamicObject>;
type AdmissionResponse = kube::core::admission::AdmissionResponse;
type AdmissionReview = kube::core::admission::AdmissionReview<DynamicObject>;
fn parse_spec<T: DeserializeOwned>(req: AdmissionRequest) -> Result<(String, String, T)> {
Copy link
Member

Choose a reason for hiding this comment

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

hmm, i don't love returning a (String, String, T) instead of a struct with named fields, but since this is only called in one place, it's probably not worth introducing a whole new type just for that...probably best the way it is...

@olix0r olix0r merged commit c445c72 into main Mar 16, 2022
@olix0r olix0r deleted the ver/ipnet-schemars branch March 16, 2022 21:03
olix0r added a commit that referenced this pull request Mar 31, 2022
`ServerAuthorization` resources are not validated by the admission
controller.

This change enables validation for `ServerAuthorization` resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The `ServerAuthoriation` validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new _schemars_ feature of `ipnet` v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould <ver@buoyant.io>
(cherry picked from commit c445c72)
Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit that referenced this pull request Mar 31, 2022
`ServerAuthorization` resources are not validated by the admission
controller.

This change enables validation for `ServerAuthorization` resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The `ServerAuthoriation` validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new _schemars_ feature of `ipnet` v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould <ver@buoyant.io>
(cherry picked from commit c445c72)
Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit that referenced this pull request Apr 7, 2022
`ServerAuthorization` resources are not validated by the admission
controller.

This change enables validation for `ServerAuthorization` resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The `ServerAuthoriation` validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new _schemars_ feature of `ipnet` v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould <ver@buoyant.io>
(cherry picked from commit c445c72)
Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit that referenced this pull request Apr 8, 2022
`ServerAuthorization` resources are not validated by the admission
controller.

This change enables validation for `ServerAuthorization` resources,
based on changes to the admission controller proposed as a part of
#8007. This admission controller is generalized to
support arbitrary resource types. The `ServerAuthoriation` validation
currently only ensures that network blocks are valid CIDRs and that they
are coherent. We use the new _schemars_ feature of `ipnet` v2.4.0 to
support using IpNet data structures directly in the custom resource
type bindings.

This change also adds an integration test to validate that the admission
controller behaves as expected.

Signed-off-by: Oliver Gould <ver@buoyant.io>
(cherry picked from commit c445c72)
Signed-off-by: Oliver Gould <ver@buoyant.io>
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.

3 participants