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

Introduction of APIRule v1beta2 with noAuth and JWT handlers #986

Closed
38 tasks done
Tracked by #939
strekm opened this issue Apr 9, 2024 · 0 comments
Closed
38 tasks done
Tracked by #939

Introduction of APIRule v1beta2 with noAuth and JWT handlers #986

strekm opened this issue Apr 9, 2024 · 0 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@strekm
Copy link
Collaborator

strekm commented Apr 9, 2024

Description

Based on accepted API proposal and POC covering technical details introduce new version of APIRule CRD. Introduced version will not be stored version. Users still have possibility to create v1beta1, that is still stored version.

New v1beta2 version should introduce 2 handlers: noAuth and JWT. noAuth handler is already released with v1beta1, so conversion is possible both ways. Also logic of noAuth is no different. JWT also exists in v1beta1 version but logic is based on ORY Oathkeeper. in v1beta2 logic behind JWT should be purely Istio based.

In case of Istio based JWT additional validation should be implemented enforcing sidecar injection on a workload.

We decided to have v1beta1 as storage version having preserveUnknownFields for v1beta2 spec fields. We keep v1beta2 clean without preserveUnknownFields. After customer manually adapted all APIRules we switch storage version. Migration step will be needed before dropping v1beta1.

Open questions to consider:

  • think about if we introduce conversion webhook as in separate container and not integrated into the operator like previously
  • think about if there is better solution for handling certificate needed than previously with the cronjob

TODOs:
@werdes72

  • api/v1beta2
  • conversion functions
  • unit tests
  • annotation should default jwt handler to istio
  • review refinements
  • CEL validation

@videlov

  • implement solution for certificates generation/renewal integrated into the api-gateway operator
  • discuss in team and get feedback
  • add unit tests
  • add envtest - not needed reconciliation functionality is fully tested with a unit-test and fake client, may introduce in the future
  • refactor conversion a bit
  • prepare integration test for v1beta1/v1beta2 conversion (ensure conversion is working)
  • support implementation on v1beta2 reconciliation flow - we will take on this in the next sprint since bigger refactoring is needed, for now we use the v1beta2 annotation to enable Istio-based JWT independent from the ConfigMap
  • review refinements
  • documentation for new controller
  • implement/workaround creating of certificate before operator manager starts - did not work out
  • ADR for Certificate controller
  • clarify initial certificate in module manifest with @TorstenD-SAP
  • introduce init-container for handling initialising certificate before manager container starts
  • workaround issue with immutable manifest and KLM apply strategy - https://sap-btp.slack.com/archives/C042CAZDZDX/p1715155956516309 - work with dynamic secret created in init-container and impl cert getter in the secret reconciler to replace file cert watcher
  • add envtest integration test for init-container initialisation functions
  • cover deletion for the certificate secret with an owner ref to api-gateway-controller-manager deployment

ACs:

  • APIRule v1beta2 introduced
  • noAuth handler present
  • Istio JWT handler executed only on v1beta2
  • ORY Oathkeeper JWT handler executed only on v1beta1
  • JWT feature toggle stays unchanged
  • integration test added / updated
  • v1beta2 spec documented

Reasons

Introduction of stable APIRule

DoD:

  • Provide unit and integration tests.
  • Provide documentation.
  • Verify if the solution works for both open-source Kyma and SAP BTP, Kyma runtime.
  • If you changed the resource limits, explain why it was needed.
  • Verify that your contributions don't decrease code coverage. If they do, explain why this is the case.
  • Add release notes.

Attachments
part of: #939
#940
#970

PRs:

@strekm strekm added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 9, 2024
@strekm strekm added this to the 2.4 milestone Apr 9, 2024
@strekm strekm mentioned this issue Apr 9, 2024
18 tasks
@werdes72 werdes72 self-assigned this Apr 15, 2024
@videlov videlov self-assigned this Apr 15, 2024
@barchw barchw assigned barchw and unassigned barchw Apr 30, 2024
@triffer triffer self-assigned this May 7, 2024
@triffer triffer removed their assignment May 15, 2024
@strekm strekm closed this as completed May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants