-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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] Modify entrypoint to sign the results. #5676
Conversation
/kind feature |
/assign pxp928 |
@jagathprakash: GitHub didn't allow me to assign the following users: pxp928. Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
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. |
/assign @pxp928 |
@jagathprakash: GitHub didn't allow me to assign the following users: pxp928. Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
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. |
/assign @lumjjb |
@jagathprakash: GitHub didn't allow me to assign the following users: lumjjb. Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
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. |
/assign @afrittoli |
The following is the coverage report on the affected files.
|
Thanks! Will take a look soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks!
/lgtm
/test check-pr-has-kind-label |
@jagathprakash: The specified target(s) for
The following commands are available to trigger optional jobs:
Use In response to this:
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. |
/test check-pr-has-kind-label |
@jagathprakash: The specified target(s) for
The following commands are available to trigger optional jobs:
Use In response to this:
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. |
/approve @jagathprakash I noticed you didn't check off the 'docs' item in the checklist but it looks like there are existing docs for the entrypoint binary (https://github.com/tektoncd/pipeline/blob/main/cmd/entrypoint/README.md) does it make sense to add some docs there? Adding a hold in case there is something to add there, feel free to remove it: /hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bobcatfish, lumjjb, pxp928, wlynch 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 |
Breaking down PR tektoncd#4759 originally proposed by @pxp928 to address TEP-0089 according @lumjjb suggestions. Plan for breaking down PR is PR 1.1: api PR 1.2: entrypointer (+cmd line + test/entrypointer) Entrypoint takes results and signs the results (termination message). PR 1.3: reconciler + pod + cmd/controller + integration tests Controller will verify the signed result. This commit corresponds to 1.2 above.
3b4e04e
to
d3ef6f8
Compare
The following is the coverage report on the affected files.
|
This will be an internal feature.
Added documentation at https://github.com/tektoncd/pipeline/blob/main/cmd/entrypoint/README.md. |
/hold cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
tamper the results and go undetected. | ||
- `-spire_socket_path`: This flag makes sense only when enable_spire is set. | ||
When enable_spire is set, spire_socket_path is used to point to the | ||
SPIRE agent socket for SPIFFE workload API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagathprakash you mentioned that the list here is incomplete - i wonder in the long run if it would make the most sense to avoid listing the individual commands here and just talk more generally about the features (and rely on the docs for the command itself vs trying to keep this in sync) - e.g. a list of features that the entrypoint binary supports such as signing with spire, etc. vs. specifics about these flags. In that case I could see there being a small section about SPIRE below. Anyway this is a good start in at least having some docs.
Breaking down PR #4759 originally proposed by @pxp928 to address TEP-0089 according @lumjjb suggestions.
Plan for breaking down PR is
PR 1.1: api
PR 1.2: entrypointer (+cmd line + test/entrypointer) Entrypoint takes results and signs the results (termination message). PR 1.3: reconciler + pod + cmd/controller + integration tests Controller will verify the signed result.
This commit corresponds to 1.2 above.
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes