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

ROX-23812: Add the acs-determine-image-tag task for ACS build pipelines #1282

Conversation

tommartensen
Copy link
Contributor

@tommartensen tommartensen commented Aug 12, 2024

Description

acs-determine-image-tag Task is one of the custom tasks we (ACS) team use in our build pipeline.
It runs a script in our repository to see if there are modified files in the workspace (fail-build-if-git-is-dirty.sh).
Then, depending on the IMAGE_TAG_STYLE, it runs the correct make tag command.
The resulting string is captured as output for use in following tasks.

The existing task is at https://github.com/stackrox/stackrox/blob/master/.tekton/determine-image-tag-task.yaml

Validation

Validated in stackrox/stackrox#12334, e.g. https://console.redhat.com/application-pipeline/ns/rh-acs-tenant/pipelinerun/central-db-build-sd8b9 and https://console.redhat.com/application-pipeline/workspaces/rh-acs/applications/acs/taskruns/central-db-build-sd8b9-determine-image-tag.

I used tkn bundle push quay.io/tommartensen/rhtap-tasks:acs-determine-image-tag -f acs-determine-image-tag.yaml to build the Task bundle for the validation.

@tommartensen
Copy link
Contributor Author

Questions for reviewers: which annotations should we set? Saw different styles used in the repository.

@tommartensen tommartensen marked this pull request as ready for review August 12, 2024 09:51
.konflux/scripts/fail-build-if-git-is-dirty.sh
image_tag=""
image_tag_style="$(params.IMAGE_TAG_STYLE)"
case "$image_tag_style" in
Copy link
Contributor

@msugakov msugakov Aug 12, 2024

Choose a reason for hiding this comment

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

Please swap image tag style for directory where to run make: stackrox/stackrox#12350

@@ -0,0 +1,23 @@
# acs-determine-image-tag task
Copy link
Contributor

Choose a reason for hiding this comment

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

Obviously, it would be good to find out a convention suggested by the Konflux team about the way to store tenant-specific trusted tasks so that they don't show up in the list of shared tasks and don't confuse other tenants. Here, additionally, the task starts with acs- just as well as the existing acs-deploy-check, acs-image-check and acs-image-scan. The thing is that acs-determine-image-tag in this PR is tenant-specific one which we need in order to have our ACS builds compliant, the other three are shared tasks that any tenant can use to leverage ACS features in their pipeline. The fact that they all will sit in the same directory with the same acs- prefix could be confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, we don't want these in build-definitions at all. It should now be possible to put custom, "untrusted" tasks in your pipeline and pass EC checks as long as those tasks do not modify the source code on the way to the build task.

That requires using a Trusted Artifacts-based pipeline. Example pipeline here: https://github.com/konflux-ci/olm-operator-konflux-sample/blob/main/.tekton/single-arch-build-pipeline.yaml

@konflux-ci/mota could probably provide more info.

Copy link
Contributor

Choose a reason for hiding this comment

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

This task does not modify the source code. It gets the source code on the input and outputs an image tag that we put on the resulting images.

We already use oci-ta tasks and not using workspaces to pass the source code around.

We, however, have some other tasks that download blobs that are included in the source code and then included in the resulting containers.

What would be the plan? Should we skip this task but open PRs for the others that download blobs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we skip this task but open PRs for the others that download blobs?

If it's generically re-usable, that may be reasonable. But the blobs downloaded by such a task would likely bypass source containers and SBOMs, wouldn't they?

The prefetch task should eventually gain the ability to download arbitrary blobs (while also allowing one to reference the source code for those blobs): https://issues.redhat.com/browse/KONFLUX-2390

Copy link
Contributor

Choose a reason for hiding this comment

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

Until then, you might as well make your build non-hermetic

Copy link
Contributor

@msugakov msugakov Aug 13, 2024

Choose a reason for hiding this comment

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

the blobs downloaded by such a task would likely bypass source containers and SBOMs, wouldn't they?

During ACS CPaaS onboarding, we had a session or two about these blobs. I could find this doc but I think there should be more records and I can dig it out if needed.
Basically, we concluded that since these blobs carry data and no code, we can skip them from source containers.
In present Konflux time, the arrangements might be different and it seems Curlito could fit us.

https://issues.redhat.com/browse/KONFLUX-2390

Thanks, I subscribed to it and linked to ACS Enablement ticket (KONFLUX-258).

@chmeliik
Copy link
Contributor

chmeliik commented Aug 13, 2024

Since you already use Trusted Artifacts (#1282 (comment)), you should be able to add custom tasks without compromising the trust of your pipeline (and EC verification should pass). Can we close this PR?

@tommartensen
Copy link
Contributor Author

Retracting this PR, because determine-image-tag is not on the critical path and not mentioned in the EC violations.

@tommartensen tommartensen deleted the tm/acs-add-determine-image-tag-task branch August 21, 2024 13:09
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