test: add e2e for conjur csi provider #1498

gl-johnson · 2024-04-17T14:40:14Z

What type of PR is this?

What this PR does / why we need it:
Adds e2e test retrieving secrets via the Conjur provider

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #

Special notes for your reviewer:

TODOs:

squashed commits
includes documentation (Follow-up PR docs: Add Conjur provider #1521)
adds unit tests

linux-foundation-easycla · 2024-04-17T14:40:18Z

The committers listed above are authorized under a signed CLA.

✅ login: gl-johnson / name: Glen Johnson (9eedbee)

k8s-ci-robot · 2024-04-17T14:40:23Z

Welcome @gl-johnson!

It looks like this is your first PR to kubernetes-sigs/secrets-store-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/secrets-store-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-04-17T14:40:23Z

Hi @gl-johnson. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bridgetkromhout · 2024-05-07T17:13:47Z

/assign @aramase @nilekhc

aramase

/ok-to-test

aramase · 2024-05-09T21:12:36Z

/test pull-secrets-store-csi-driver-e2e-conjur

codecov-commenter · 2024-05-09T21:51:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.78%. Comparing base (bf86dbf) to head (9eedbee).
Report is 21 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1498      +/-   ##
==========================================
- Coverage   36.58%   35.78%   -0.81%     
==========================================
  Files          63       63              
  Lines        4532     3759     -773     
==========================================
- Hits         1658     1345     -313     
+ Misses       2730     2270     -460     
  Partials      144      144

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gl-johnson · 2024-05-20T17:46:45Z

Hi @aramase @nilekhc - any chance this can be reviewed this week? Let me know if you have any feedback/questions. Thanks!

aramase · 2024-05-23T17:23:06Z

test/bats/conjur.bats

+@test "allow conjur provider token requests and enable rotation" {
+  helm upgrade csi-secrets-store manifest_staging/charts/secrets-store-csi-driver --namespace=kube-system \
+    --set tokenRequests[0].audience="conjur" \
+		--set enableSecretRotation=true \
+		--set rotationPollInterval=30s
+
+  kubectl wait --for=condition=ready --timeout=${WAIT_TIME}s --namespace=kube-system pod -l "app=secrets-store-csi-driver"
+}


Could you add --set tokenRequests[2].audience="conjur" to

secrets-store-csi-driver/Makefile

Lines 439 to 440 in 5f6daa0

--set tokenRequests[0].audience="aud1" \

--set tokenRequests[1].audience="aud2"

instead and remove this step?

aramase · 2024-05-23T17:25:03Z

test/bats/conjur.bats

+  # Create Conjur namespace
+  kubectl create namespace $CONJUR_NAMESPACE 


You can use --create-namespace flag as part of helm install command

aramase · 2024-05-23T17:25:58Z

test/bats/conjur.bats

+
+  # Install Conjur CSI Provider
+  helm install conjur-csi-provider cyberark/conjur-k8s-csi-provider --wait --timeout ${WAIT_TIME}s --namespace=kube-system \
+    --set providerServer.image.tag=latest


latest tag in provider could break CI in the driver. Is there a pinned version you can use here?

aramase · 2024-05-23T17:27:15Z

test/bats/conjur.bats

+  docker pull cyberark/conjur-cli:8
+  kind load docker-image cyberark/conjur-cli:8


The image is public, so I don't think this step is required.

aramase · 2024-05-23T17:29:28Z

test/bats/conjur.bats

+}
+
+@test "CSI inline volume test with pod portability" {
+  kubectl apply --namespace=kube-system -f $BATS_TESTS_DIR/pod-secrets-store-inline-volume-crd.yaml


Does the workload need to be in kube-system namespace? If someone looks at this file for reference, I don't want them to assume this is a requirement. Could we use any namespace other than kube-system for the workloads being deployed?

I've moved the test workloads to the default namespace

gl-johnson · 2024-05-23T19:08:59Z

/test pull-secrets-store-csi-driver-e2e-conjur

gl-johnson · 2024-05-23T20:21:28Z

/retest-required

aramase

Just one minor comment!

After the update, could you squash to a single commit?

aramase · 2024-05-23T21:41:45Z

test/bats/conjur.bats

+  helm install conjur cyberark/conjur-oss --namespace=$CONJUR_NAMESPACE --wait --timeout ${WAIT_TIME}s \
+    --create-namespace --namespace=$CONJUR_NAMESPACE \


Suggested change

helm install conjur cyberark/conjur-oss --namespace=$CONJUR_NAMESPACE --wait --timeout ${WAIT_TIME}s \

--create-namespace --namespace=$CONJUR_NAMESPACE \

helm install conjur cyberark/conjur-oss --namespace=$CONJUR_NAMESPACE --wait --timeout ${WAIT_TIME}s \

--create-namespace \

--namespace flag is already defined

Woops - all should be addressed now!

gl-johnson · 2024-05-23T22:56:31Z

/retest-required

gl-johnson · 2024-05-24T14:42:39Z

/retest-required

aramase · 2024-05-24T16:23:09Z

/test pull-secrets-store-csi-driver-e2e-conjur

aramase

/lgtm
/approve

k8s-ci-robot · 2024-05-24T16:33:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aramase, gl-johnson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [aramase]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 17, 2024

k8s-ci-robot requested review from aramase and ritazh April 17, 2024 14:40

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 17, 2024

gl-johnson force-pushed the conjur-e2e branch 2 times, most recently from 135d1d1 to d1d197f Compare April 18, 2024 15:52

gl-johnson marked this pull request as ready for review April 18, 2024 15:53

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 18, 2024

gl-johnson force-pushed the conjur-e2e branch from 69d61f2 to 09f3cf3 Compare April 23, 2024 14:56

k8s-ci-robot assigned aramase and nilekhc May 7, 2024

gl-johnson mentioned this pull request May 9, 2024

chore: add optional presubmit for conjur provider with secrets-store-csi kubernetes/test-infra#32573

Merged

aramase reviewed May 9, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 9, 2024

gl-johnson requested a review from aramase May 14, 2024 14:33

imheresamir mentioned this pull request May 16, 2024

docs: Add Conjur provider #1521

Open

3 tasks

aramase requested changes May 23, 2024

View reviewed changes

gl-johnson requested a review from aramase May 23, 2024 20:59

aramase reviewed May 23, 2024

View reviewed changes

Add conjur e2e bats tests

9eedbee

gl-johnson force-pushed the conjur-e2e branch from fc8d9cc to 9eedbee Compare May 23, 2024 22:34

aramase reviewed May 24, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2024

k8s-ci-robot merged commit f95f32d into kubernetes-sigs:main May 24, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add e2e for conjur csi provider #1498

test: add e2e for conjur csi provider #1498

gl-johnson commented Apr 17, 2024 •

edited

linux-foundation-easycla bot commented Apr 17, 2024 •

edited

k8s-ci-robot commented Apr 17, 2024

k8s-ci-robot commented Apr 17, 2024

bridgetkromhout commented May 7, 2024

aramase left a comment

aramase commented May 9, 2024

codecov-commenter commented May 9, 2024 •

edited

gl-johnson commented May 20, 2024

aramase May 23, 2024

aramase May 23, 2024

aramase May 23, 2024

aramase May 23, 2024

aramase May 23, 2024

gl-johnson May 23, 2024 •

edited

gl-johnson commented May 23, 2024

gl-johnson commented May 23, 2024

aramase left a comment

aramase May 23, 2024

gl-johnson May 23, 2024

gl-johnson commented May 23, 2024

gl-johnson commented May 24, 2024

aramase commented May 24, 2024

aramase left a comment

k8s-ci-robot commented May 24, 2024

	--set tokenRequests[0].audience="aud1" \
	--set tokenRequests[1].audience="aud2"

		# Create Conjur namespace
		kubectl create namespace $CONJUR_NAMESPACE

		docker pull cyberark/conjur-cli:8
		kind load docker-image cyberark/conjur-cli:8

		helm install conjur cyberark/conjur-oss --namespace=$CONJUR_NAMESPACE --wait --timeout ${WAIT_TIME}s \
		--create-namespace --namespace=$CONJUR_NAMESPACE \

test: add e2e for conjur csi provider #1498

test: add e2e for conjur csi provider #1498

Conversation

gl-johnson commented Apr 17, 2024 • edited

linux-foundation-easycla bot commented Apr 17, 2024 • edited

k8s-ci-robot commented Apr 17, 2024

k8s-ci-robot commented Apr 17, 2024

bridgetkromhout commented May 7, 2024

aramase left a comment

Choose a reason for hiding this comment

aramase commented May 9, 2024

codecov-commenter commented May 9, 2024 • edited

Codecov Report

gl-johnson commented May 20, 2024

aramase May 23, 2024

Choose a reason for hiding this comment

aramase May 23, 2024

Choose a reason for hiding this comment

aramase May 23, 2024

Choose a reason for hiding this comment

aramase May 23, 2024

Choose a reason for hiding this comment

aramase May 23, 2024

Choose a reason for hiding this comment

gl-johnson May 23, 2024 • edited

Choose a reason for hiding this comment

gl-johnson commented May 23, 2024

gl-johnson commented May 23, 2024

aramase left a comment

Choose a reason for hiding this comment

aramase May 23, 2024

Choose a reason for hiding this comment

gl-johnson May 23, 2024

Choose a reason for hiding this comment

gl-johnson commented May 23, 2024

gl-johnson commented May 24, 2024

aramase commented May 24, 2024

aramase left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 24, 2024

gl-johnson commented Apr 17, 2024 •

edited

linux-foundation-easycla bot commented Apr 17, 2024 •

edited

codecov-commenter commented May 9, 2024 •

edited

gl-johnson May 23, 2024 •

edited