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

Integration test #52

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

lmilleri
Copy link
Member

@lmilleri lmilleri commented Oct 14, 2024

Run an e2e test with the sample attester in an ephemeral kind cluster.
This test requires the k8s kuttl plugin and kind installation when executed in local (e.g. make test-e2e).
Also added github action for executing the test on PR and merge in the main branch

@lmilleri lmilleri requested a review from a team as a code owner October 14, 2024 10:45
@lmilleri lmilleri marked this pull request as draft October 14, 2024 15:10
@lmilleri lmilleri force-pushed the integration-test branch 3 times, most recently from 8a7984c to 5eac361 Compare October 15, 2024 07:53
@lmilleri lmilleri marked this pull request as ready for review October 15, 2024 07:54
@lmilleri
Copy link
Member Author

PR reshaped to use the kuttl plugin, please review

@spotlesstofu
Copy link

Can you make this run on Github?

README.md Outdated

An attestation with the sample-attester is performed in an ephemeral kind cluster
Pre-requirements:
- k8s kuttl plugin installed

Choose a reason for hiding this comment

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

Please add a link to how to do this.

@@ -0,0 +1,3 @@
#!/bin/bash

oc create secret generic kbsres1 --from-literal key1=res1val1 --from-literal key2=res1val2 -n trustee-operator-system
Copy link
Member

Choose a reason for hiding this comment

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

s/oc/kubectl

Makefile Outdated
# pre-requirements: k8s kuttl plugin installed
# you can change the trustee image by defining the env variable KBS_IMAGE_NAME
# Usage: REGISTRY=<user-registry> KBS_IMAGE_NAME=<trustee-image> make e2e-test
.PHONY: e2e-test
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/e2e-test/test-e2e

@lmilleri lmilleri marked this pull request as draft October 17, 2024 16:03
@lmilleri lmilleri marked this pull request as ready for review October 17, 2024 16:30
@lmilleri
Copy link
Member Author

Addressed comments and added github action.
Github action can't be tested right now, it'll be active when this PR is merged (tested in my personal repo though).
Please review

@@ -0,0 +1,21 @@
#!/bin/bash

KBS_IMAGE_NAME="${KBS_IMAGE_NAME:-quay.io/confidential-containers/trustee:290fd0eb64ab20f50efbd27cf80542851c0ee17f}"

Choose a reason for hiding this comment

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

Remove the default value for this variable, we want the test to fail if it's not set.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'd need both the trustee and the client images to be specified and they are mandatory for the test to execute. I'll do the changes for the client image customization.
I think the default images have to be provided somehow, otherwise the github action won't be able to execute properly. Where do you reckon these images should be hardcoded?

  1. github action: ok for CI but if you run make test-e2e the images must be provided in the command
  2. Makefile: both CI and make test-e2e are ok

Choose a reason for hiding this comment

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

Makefile

Run an e2e test with ephemeral kind cluster.
Usage: make test-e2e
The integration test is also executed as a github action

Signed-off-by: Leonardo Milleri <[email protected]>
Copy link
Member

@bpradipt bpradipt left a comment

Choose a reason for hiding this comment

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

/lgtm
Nice work @lmilleri

@bpradipt bpradipt merged commit b71d3a3 into confidential-containers:main Oct 22, 2024
3 checks passed
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.

3 participants