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

[TEP-0089] SPIRE for non-falsifiable provenance. Setup the test environment. #6553

Merged
merged 1 commit into from
May 4, 2023

Conversation

jagathprakash
Copy link
Member

@jagathprakash jagathprakash commented Apr 18, 2023

[TEP-0089] SPIRE for non-falsifiable provenance. Setup the test environment.

This PR is part of a set of PRs to enable non-falsifiable provenance using SPIRE. This PR sets up the environment needed to enable this feature in E2E tests. Note that the SPRIRE flag itself is not enabled, i.e. the feature itself is not enabled. This PR is to test if adding the SPIRE environment does not break anything in E2E tests.

Changes

This PR introduces the environment needed to run SPIRE in E2E tests.
Note that the SPIRE feature itself is not enabled.

A tracking bug for this feature, which includes all the PRs added till date, is at
#6597

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

@jagathprakash
Copy link
Member Author

/kind feature

@tekton-robot tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note-none Denotes a PR that doesnt merit a release note. labels Apr 18, 2023
@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 18, 2023
@jagathprakash jagathprakash marked this pull request as draft April 18, 2023 15:30
@tekton-robot tekton-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 18, 2023
@jagathprakash jagathprakash marked this pull request as ready for review May 1, 2023 18:25
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 1, 2023
@jagathprakash
Copy link
Member Author

/retest

@jagathprakash
Copy link
Member Author

/retest-required

Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

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

thank you @jagathprakash!

test/e2e-tests.sh Outdated Show resolved Hide resolved
test/e2e-common.sh Show resolved Hide resolved
test/e2e-common.sh Outdated Show resolved Hide resolved
test/e2e-common.sh Show resolved Hide resolved
@jagathprakash
Copy link
Member Author

Thanks @jerop for the review. Addressed all the comments.

@jagathprakash
Copy link
Member Author

/retest-required

…onment.

This PR is part of a set of PRs to enable non-falsifiable provenance using SPIRE.
This PR sets up the environment needed to enable this feature in E2E tests.
Note that the SPRIRE flag itself is not enabled, i.e. the feature itself is not enabled.
This PR is to test if adding the SPIRE environment does not break anything in E2E tests.

Signed-off-by: jagathprakash <31057312+jagathprakash@users.noreply.github.com>
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

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:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 3, 2023
Copy link
Member

@chuangw6 chuangw6 left a comment

Choose a reason for hiding this comment

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

Thanks @jagathprakash !
Can we clean up the PR description a bit? i.e. having a single place to explain the changes, and removing links to the previous individual PRs b/c #6597 includes them all already

@@ -91,6 +103,7 @@ function run_e2e() {
fi
}

add_spire "$PIPELINE_FEATURE_GATE"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this line come after set_feature_gate "$PIPELINE_FEATURE_GATE"?

Copy link
Member Author

Choose a reason for hiding this comment

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

The order does not matter as we are not depending on whether spire feature is enabled or not to install SPIRE, which is what add_spire() does.

Copy link
Member

Choose a reason for hiding this comment

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

But add_spire only installs spire when the alpha feature flag is enabled right? set_feature_gate "$PIPELINE_FEATURE_GATE" is the place where we set alpha feature flag for integration test. If add_spire is called before set_feature_gate, it seems to me the if condition in add_spire will never be met?

Copy link
Member Author

Choose a reason for hiding this comment

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

$PIPELINE_FEATURE_GATE is an env variable which is passed in into the e2e-tests.sh script (or rather set before e2e-tests.sh). This can be alpha or stable.
We set the feature flags based on this env variable.
Both add_spire and set_feature_gate depend on this env variable. set_feature_gate sets the feature_flags config based on this env variable.

Copy link
Member

Choose a reason for hiding this comment

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

aha you're right, my bad. I don't why I ignored the fact the add_spire uses $PIPELINE_FEATURE_GATE as a parameter :/

Thank you for explaining it.

@jagathprakash
Copy link
Member Author

Thanks @jagathprakash ! Can we clean up the PR description a bit? i.e. having a single place to explain the changes, and removing links to the previous individual PRs b/c #6597 includes them all already

Done.

@chuangw6
Copy link
Member

chuangw6 commented May 4, 2023

/lgtm

1 similar comment
@chuangw6
Copy link
Member

chuangw6 commented May 4, 2023

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 4, 2023
@tekton-robot tekton-robot merged commit add97ea into tektoncd:main May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesnt merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants