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

Introduce AuthorizationPolicy CRDs #8007

Merged
merged 10 commits into from
Mar 30, 2022
Merged

Introduce AuthorizationPolicy CRDs #8007

merged 10 commits into from
Mar 30, 2022

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Mar 4, 2022

Issue #7709 proposes new Custom Resource types to support generalized
authorization policies:

  • AuthorizationPolicy
  • MeshTLSAuthentication
  • NetworkAuthentication

This change introduces these CRDs to the default linkerd installation
(via the linkerd-crds chart) and updates the policy controller's
to handle these resource types. The policy admission controller
validates that these resource reference only suppported types.

This new functionality is tested at multiple levels:

  • linkerd-policy-controller-k8s-index includes unit tests for the
    indexer to test how events update the index;
  • linkerd-policy-test includes integration tests that run in-cluster
    to validate that the gRPC API updates as resources are manipulated;
  • linkerd-policy-test includes integration tests that exercise the
    admission controller's resource validation; and
  • linkerd-policy-test includes integration tests that ensure that
    proxies honor authorization resources.

This change does NOT update Linkerd's control plane and extensions to
use these new authorization primitives. Furthermore, the linkerd CLI
does not yet support inspecting these new resource types. These
enhancements will be made in followup changes.

@olix0r olix0r force-pushed the ver/authz-crd branch 30 times, most recently from 9aac732 to 8fb5b02 Compare March 5, 2022 22:19
olix0r added a commit that referenced this pull request 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>
olix0r added a commit that referenced this pull request 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>
@olix0r olix0r marked this pull request as ready for review March 29, 2022 20:22
@olix0r olix0r requested a review from a team as a code owner March 29, 2022 20:22
Issue #7709 proposes new Custom Resource types to support generalized
authorization policies:

- `AuthorizationPolicy`
- `MeshTLSAuthentication`
- `NetworkAuthentication`

This change introduces these CRDs to the default linkerd installation
(via the `linkerd-crds` chart) and updates the policy controller's
to handle these resource types. The policy admission controller
validates that these resource reference only suppported types.

This new functionality is tested at multiple levels:

* `linkerd-policy-controller-k8s-index` includes unit tests for the
  indexer to test how events update the index;
* `linkerd-policy-test` includes integration tests that run in-cluster
  to validate that the gRPC API updates as resources are manipulated;
* `linkerd-policy-test` includes integration tests that exercise the
  admission controller's resource validation; and
* `linkerd-policy-test` includes integration tests that ensure that
  proxies honor authorization resources.

This change does NOT update Linkerd's control plane and extensions to
use these new authorization primitives. Furthermore, the `linkerd` CLI
does not yet support inspecting these new resource types. These
enhancements will be made in followup changes.

Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

Overall this looks good. There's a decent amount of test code that will become more clear after maybe writing some tests myself, but the CRD changes along with the indexing all make sense.

.filter(|authn| authn.targets_kind::<MeshTLSAuthentication>())
.count();
if mtls_authns_count > 1 {
bail!("only a single MeshTLSAuthentication may be set");
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense but was not immediately obvious. I don't think it's a lack of documentation, more just leaving this comment as an explanation to myself. In order for multiple identities to be authorized by an AuthorizationPolicy, we only can select at most one MeshTLSAuthentication. This MeshTLSAuthentication can select multiple identities though, so all those identities will be authorized clients to whatever the targetRef is.

policy-controller/k8s/index/src/index.rs Outdated Show resolved Hide resolved
Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
Signed-off-by: Oliver Gould <ver@buoyant.io>
…tus detected to free resources

Signed-off-by: Oliver Gould <ver@buoyant.io>
@olix0r olix0r merged commit c1a1430 into main Mar 30, 2022
@olix0r olix0r deleted the ver/authz-crd branch March 30, 2022 19:26
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.

4 participants