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

AuthConfig conversion webhook #137

Merged
merged 6 commits into from
Sep 19, 2023
Merged

AuthConfig conversion webhook #137

merged 6 commits into from
Sep 19, 2023

Conversation

guicassolato
Copy link
Collaborator

@guicassolato guicassolato commented Aug 29, 2023

Deploys the AuthConfig CRD conversion webhook (based on the Authorino container image), as part of deploying the Operator.

This is because the conversion webhook is a single deployment per cluster (similarly to the Operator itself), nevertheless it's based on the Authorino code base (which owns the AuthConfig type and therefore the functions to convert between versions of the CRD).

Important!

This change introduces a new dependency of the Operator on cert-manager. Actually the dependency is introduced by the conversion webhook (not by the Operator itself), but since installing/deploying the Operator will also deploy the webhook, cert-manager needs to be installed before installing the Operator.

Because of that, installing the Operator by simply applying the config/deploy manifests may no longer work, with the webhook possibly failing due to cert-manager not running in the cluster. Hence, the PR also introduces a script to continue supporting installing the Operator – and now also cert-manager – without having to clone the repo nor depending on Operator Lifecycle Manager (OLM).

The script can be used as follows:

curl -sL https://github.com/raw/Kuadrant/authorino-operator/main/utils/install.sh | bash -s -- [options]

The options include : --version, --git-ref, --no-deps, and --dry-run.

For installations via OLM, installing cert-manager first is not required.


Related to Kuadrant/authorino#417

Deploys the AuthConfig CRD conversion webhook (based on the Authorino container image), as part of deploying the Operator.

This is because the conversion webhook is a single deployment per cluster (similarly to the Operator itself), nevertheless it's based on the Authorino code base (which owns the AuthConfig type and therefore the functions to convert between versions of the CRD).

This change introduces a dependency of the Operator on cert-manager (https://cert-manager.io).
@guicassolato guicassolato marked this pull request as ready for review August 31, 2023 08:24
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Merging #137 (cd68d75) into main (b03fcf4) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #137   +/-   ##
=======================================
  Coverage   63.11%   63.11%           
=======================================
  Files           1        1           
  Lines         732      732           
=======================================
  Hits          462      462           
  Misses        220      220           
  Partials       50       50           
Flag Coverage Δ
unit 63.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@guicassolato
Copy link
Collaborator Author

The Verify manifests test failing is expected, because Kuadrant/authorino#417 has not been merged yet.

@guicassolato guicassolato requested a review from a team August 31, 2023 08:50
@guicassolato guicassolato mentioned this pull request Aug 31, 2023
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Sep 4, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137
Boomatang
Boomatang previously approved these changes Sep 4, 2023
Copy link
Contributor

@Boomatang Boomatang left a comment

Choose a reason for hiding this comment

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

Running the script worked as expected. By using the --git-ref flag I a was able to install from this branch and all the web hook got deployed as expected.

Copy link
Member

@adam-cattermole adam-cattermole left a comment

Choose a reason for hiding this comment

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

I verified these changes and the script standalone (the creation/deletion of authconfig v1beta2 with the webhook server), as well as alongside the changes in authorino through running the user guides and everything looks good to me

Copy link
Contributor

@didierofrivia didierofrivia left a comment

Choose a reason for hiding this comment

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

Works as expected! using the script provided in this branch with --git-ref conversion-webhook

@didierofrivia
Copy link
Contributor

Ok, something I've just seen when installing it with curl -sL https://github.com/raw/Kuadrant/authorino-operator/152a1627a0b2f5d36c3983192644ec8c5d625bad/utils/install.sh | bash -s -- --git-ref conversion-webhook --version authconfig-v1beta2 after waiting for a while, the webhook is still progressing

 k describe deployment/authorino-webhooks -n authorino-operator

...

Conditions:
  Type           Status  Reason
  ----           ------  ------
  Progressing    True    NewReplicaSetAvailable
  Available      False   MinimumReplicasUnavailable
OldReplicaSets:  <none>
NewReplicaSet:   authorino-webhooks-f77ddcc88 (1/1 replicas created)
Events:
  Type    Reason             Age    From                   Message
  ----    ------             ----   ----                   -------
  Normal  ScalingReplicaSet  4m52s  deployment-controller  Scaled up replica set authorino-webhooks-f77ddcc88 to 1

@adam-cattermole
Copy link
Member

@didierofrivia the --version is the version of authorino-operator, and not authorino (--git-ref takes precedence in your case). The webhook server will be deployed with authorino:latest (which does not support webhook server) and won't progress unless you patch the image

kubectl get deployments authorino-webhooks -n authorino-operator -o yaml | yq '.spec.template.spec.containers[0].image'
#quay.io/kuadrant/authorino:latest

Can patch like this:

kubectl patch deployment/authorino-webhooks -n authorino-operator -p '{"spec":{"template":{"spec":{"containers":[{"name":"webhooks","image":"quay.io/kuadrant/authorino:authconfig-v1beta2"}]}}}}'

@didierofrivia
Copy link
Contributor

Right! thanks @adam-cattermole , I totally misread the script annotation xD

@guicassolato guicassolato merged commit 261466d into main Sep 19, 2023
5 of 6 checks passed
@guicassolato guicassolato deleted the conversion-webhook branch September 19, 2023 15:02
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Sep 27, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Sep 27, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Sep 27, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 10, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 10, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 11, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 16, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 18, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 20, 2023
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
guicassolato added a commit to Kuadrant/kuadrant-operator that referenced this pull request Oct 20, 2023
* AuthPolicy v1beta2

Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

* Bump Authorino Operator to v0.9.0

Closes #263

* Superseding of strict host subsets between AuthConfigs

Enables [superseding of strict host subsets](https://github.com/Kuadrant/authorino/blob/main/docs/architecture.md#avoiding-host-name-collision) in Authorino – i.e., set `SupersedingHostSubsets` to `true` in the Authorino CR.

Closes #264.

* Route selectors for the AuthPolicy

* Merge the hostnames of HTTPRoute (direct or inherited from the Gateway) into all Istio AuthorizationPolicy rules that do not include hostnames already built from the route selectors, so we don't send a request to authorino for hosts that are not in the scope of the policy

* AuthConfig with OR conditions between HTTPRouteMatches of a HTTPRouteRule and between HTTPRouteRules themselves

* Fix reconciliation of gateway AuthPolicies

Skips creation of Istio AuthorizationPolicies and Authorino AuthConfigs for gateways without any accepted HTTPRoutes.

* Ensure Gateway API group is used when checking the targetRef kind

* Trigger reconciliation of possibly affected gateway policies after reconciling HTTPRoute ones

* Mapper of HTTPRoute events to policy events that goes through the parentRefs of the route and finds all policies that target one of its parent resources, thus yielding events for those policies.

* AuthConfig condition from GWAPI QueryParamMatchRegularExpression

* Skip Istio AuthorizationPolicy rules for GWAPI PathMatchRegularExpression and HeaderMatchRegularExpression

* Generate Istio AuthorizationPolicy rules out of top-level conditions or full HTTPRoute only (i.e. ignore config-level conditions)

* tests: fix integration tests for authpolicy with route selectors

* fix: AuthorizationPolicy rules when PathMatchRegularExpression

+ unit tests from Istio AuthorizationPolicy rules from HTTPRouteRules and hostnames

* tests: unit tests from AuthConfig conditions from HTTPRouteRules and hostnames

* tests: unit tests for the AuthPolicy type

* tests: unit test for RateLimitPolicyList.GetItems()

* tests: unit test for RouteSelectors.HostnamesForConditions()

* tests: integration test for Gateway policy with having other policies attached to HTTPRoutes

* fixup: AuthPolicy SuccessResponseSpec type

* tests: integration tests for AuthPolicy <-> AuthConfig mapping

* tests: integration tests gateway policies whith all HTTPRoutes talken

* Well-known attributes used in the generated AuthConfigs

Closes:
- #265

* tests: integration tests for policies only with unmatching route selectors

* lint: fix duplicated imported package

* Move AuthPolicy v1beta top-level 'patterns' and 'when' fields one level up

This will pair the level of these policy-wide options to the top-level 'routeSelectors', rather than having two things that have semantics over the same scope defined at different levels in the API.

This change also separates the auth scheme, making it now exclusively about auth rules.

* [authpolicy-v2] docs (#275)

* docs: authpolicy v1beta2

* docs: addressing suggestions of enhancements to the authpolicy docs

* Add mandatory Gateway API label to the AuthPolicy CRD (#279)

Closes #278

* docs: fix typos

* [authpolicy-v2] AuthPolicy validation rules (#281)

* prevent usage of routeSelectors in a gateway AuthPolicy

* AuthPolicy CEL validation rules

- Invalid targetRef.group
- Invalid targetRef.kind
- Route selectors not supported when targeting a Gateway

Note: cannot set a validation rule for !has(spec.targetRef.namespace) || spec.targetRef.namespace == metadata.namespace, because Kubernetes does not allow accessing `metadata.namespace`. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request size/medium
Projects
No open projects
Status: To test
Development

Successfully merging this pull request may close these issues.

5 participants