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

Add ingress test for opaAuthorizeRequest filter #8575

Open
wants to merge 49 commits into
base: dev
Choose a base branch
from
Open

Conversation

wisinghe
Copy link
Collaborator

@wisinghe wisinghe commented Dec 5, 2024

Changes

  • Skipper rbac configuration is ordered to be applied before deployment to make sure the service account is created before the deployment.

  • Add three new steps in delivery.yaml

  1. create-opa-enabled-cluster: creates a new cluster using the same configuration + manifests as the create-cluster step. The only difference in the configuration is skipper_open_policy_agent_enabled flag value. The CLUSTER_ALIAS and LOCAL_ID is adjusted so that it does not conflict with the e2e cluster that is created with create-cluster step.

  2. opa-e2e-tests: Run the e2e test for opaAuthorizeRequest filter

  3. decommission-opa-enabled-cluster: Decommissions the opa enabled cluster

  • Change test/e2e/run_e2e.sh to enable different variation of cluster_creation, e2e testing and cluster_decommissioning.

  • opaAuthorizeRequest is initialized and verified in tests

@wisinghe wisinghe added minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call. do-not-merge labels Dec 5, 2024
@wisinghe wisinghe added e2e and removed do-not-merge labels Dec 6, 2024
@zalando-robot
Copy link

Cannot start a pipeline due to:

Pipeline creation failed with: found undefined alias 'apply_env'
  in "<unicode string>", line 470, column 12:
          env: *apply_env
               ^

Click on pipeline status check Details link below for more information.

@zalando-robot
Copy link

Cannot start a pipeline due to:

Pipeline creation failed with: found undefined alias 'apply_env'
  in "<unicode string>", line 509, column 12:
          env: *apply_env
               ^

Click on pipeline status check Details link below for more information.

@zalando-robot
Copy link

Cannot start a pipeline due to:

Pipeline creation failed with: found undefined alias 'apply_env'
  in "<unicode string>", line 509, column 12:
          env: *apply_env
               ^

Click on pipeline status check Details link below for more information.

@zalando-robot
Copy link

Cannot start a pipeline due to:

Pipeline creation failed with: line 500, col 3: Unknown step id in `depends_on` value: e2e-load-test-result
line 500, col 3: Unknown step id in `depends_on` value: e2e-tests
line 500, col 3: Unknown step id in `depends_on` value: stackset-e2e-tests

Click on pipeline status check Details link below for more information.

@wisinghe wisinghe changed the title [WIP] Add ingress test for opaAuthorizeRequest filter Add ingress test for opaAuthorizeRequest filter Dec 20, 2024
@wisinghe wisinghe marked this pull request as ready for review December 20, 2024 13:56
@@ -318,6 +340,230 @@ pipeline:
cpu: 500m
memory: 1Gi

- id: create-opa-enabled-cluster
Copy link
Member

Choose a reason for hiding this comment

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

TBH I don't like that we create another cluster in the pipeline; It will add another 30m to each PR, but that's for teapot to decide.

Copy link
Member

Choose a reason for hiding this comment

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

this PR time

Total steps runtime: 1 h, 34 m, 26 s

other PR times (went through 5 not all all of them)

Total steps runtime: 1 h, 8 m, 47 s

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm.. I understand that it would be a concern. Will wait for Teapot's feedback for that. However we do not hope to keep these steps forever. It's only until we are confident enough to enable this feature for all the clusters.

Also the overall completion time for a build is not affected as these 3 steps run parallel to other create-cluster, e2e steps.

test/e2e/run_e2e.sh Outdated Show resolved Hide resolved
test/e2e/ingress.go Outdated Show resolved Hide resolved
test/e2e/ingress.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants