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 0036-trusted-artifacts #195

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Add 0036-trusted-artifacts #195

merged 1 commit into from
Jul 15, 2024

Conversation

lcarva
Copy link
Contributor

@lcarva lcarva commented Jun 11, 2024

No description provided.

@lcarva
Copy link
Contributor Author

lcarva commented Jun 11, 2024

@ralphbean, @brianwcook, @arewm PTAL 🙏

ADR/0036-trusted-artifacts.md Outdated Show resolved Hide resolved
ADR/0036-trusted-artifacts.md Outdated Show resolved Hide resolved
were properly used in the parts of the build Pipeline that affect the build outcome, e.g.
`git-clone` and `buildah` Tasks.

Trusted Artifacts is inspired by the upcoming work being done by the Tekton Community,
Copy link
Member

Choose a reason for hiding this comment

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

@lcarva so the "trusted artifacts" referenced here is different from the upcoming upstream work ? If yes, aren't we a bit afraid there might be some confusion (if it's slightly different and we have to "migrate" from one to the other).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The upstream work will require changes to the Tekton resources IIUC. I don't think there's a way to avoid a migration. But hopefully, this is isolated to Task definitions and we can switch over without requiring a migration from the user side.

location of the archive and its checksum digests are recorded as a Task result. The process of
*consuming* a Trusted Artifact extracts such archive, while verying its checksum digest, into a
volume only accessible to the Task, e.g. `emptyDir`. The name and checksum digest of the archive is
provided via Task parameters.
Copy link
Member

@ifireball ifireball Jun 14, 2024

Choose a reason for hiding this comment

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

Suggested change
provided via Task parameters.
provided via Task parameters.
With the artifact checksums being passed over the task parameters we can ensure that
the artifacts are not being tampered with as they are passed between different pipeline
tasks.

Trying to summarize our other thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TY! I reworded this a bit. Let me know what you think

Copy link
Member

@arewm arewm left a comment

Choose a reason for hiding this comment

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

The comments are all relatively minor. Thanks for writing this up!

process. This is distinct from other build systems where a rigid process prevents users from
applying such customizations. To support this, Konflux build Pipelines use Trusted Artifacts to
securely share files between Tasks. Enterprise Contract is then responsible for verifying that
Trusted Artifacts were properly used in the parts of the build Pipeline that affect the build
Copy link
Member

Choose a reason for hiding this comment

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

It also is used to verify that the contents were not tainted by untrusted tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this falls under the "Trusted Artifacts were properly used" comment.

ADR/0036-trusted-artifacts.md Outdated Show resolved Hide resolved
When the time comes, the Konflux implementation should align with what is provided by the Tekton
Community and this ADR may need a revision.

In brief, the processes of *creating* a Trusted Artifact wraps files into an archive. Then, the
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to be more specific about what the archive is? These archives also need to be tamper-proof.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really don't want this ADR to be a design doc of Trusted Artifacts. I think that belongs somewhere else. https://github.com/konflux-ci/build-trusted-artifacts is likely a good place for it.


## Decision

Sharing files between Tasks is done via Trusted Artifacts backed by OCI storage.
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to state that we cannot use the referrer's api here because these archives are generated before we produce an image? I don't know if anyone might wonder about it. (no is a fine response).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's too low level for this doc.

Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
@lcarva
Copy link
Contributor Author

lcarva commented Jul 15, 2024

@arewm, I believe I addressed all of your comments. Let me know what you think.

(BTW, I don't have merge permission here, so if you're happy with it, please merge 🙏)

@arewm arewm merged commit e088b1d into konflux-ci:main Jul 15, 2024
1 check 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.

7 participants