openshift-pipelines / pipelines-as-code

Pipelines-as-Code for Tekton
https://pipelinesascode.com
Apache License 2.0
137 stars 81 forks source link

Add changes for onboarding PAC to Konflux #1795

Closed savitaashture closed 13 hours ago

savitaashture commented 4 weeks ago

Changes

This PR integrates PAC into Konflux, an open-source, cloud-native software factory that prioritizes software supply chain security. By leveraging Konflux, PAC can now be built and released with enhanced security and consistency. Konflux ensures all images are built hermetically, prefetching dependencies in advance to eliminate runtime internet downloads. Additionally, it enforces policy checks to verify that all preconditions are satisfied, reinforcing compliance and reliability during the build process.

Signed-off-by: savitaashture sashture@redhat.com

Submitter Checklist

savitaashture commented 4 weeks ago

/test linters

chmouel commented 3 weeks ago

do we need a openshift/ directory at the topdir ? can it be in .konflux? I find it weird to have those files in a upstream repository

vdemeester commented 3 weeks ago

do we need a openshift/ directory at the topdir ? can it be in .konflux? I find it weird to have those files in a upstream repository

Today we need it (by convention πŸ˜…). We could move them elsewhere (under .konflux for example), we just need to update a bit the tool that generates the payloads…

chmouel commented 3 weeks ago

(by convention πŸ˜…). We could move them elsewhere (under .konflux for example), we just need to update a bit the tool that generates the payloads…

i'd rather have the less downstream thing directory and files upstream, i don't think those files are anything useful for the normal folks that don't ship a openshift product

vdemeester commented 3 weeks ago

i'd rather have the less downstream thing directory and files upstream, i don't think those files are anything useful for the normal folks that don't ship a openshift product

Yeah, that make sense πŸ‘ΌπŸΌ

chmouel commented 3 weeks ago

just to clarify i am fine to have a toplevel .konflux with all things downstream in there but not just many directories across the top level...

savitaashture commented 2 weeks ago

@vdemeester as per the discussion moved everything under .konflux PTAL cc @chmouel

chmouel commented 2 weeks ago

some comments are still not addressed:

vdemeester commented 2 weeks ago

can we have one Dockerfile instead of duplication?

Nope, at least not yet πŸ˜“. Those Dockerfile are written once and almost never updated (or automatically), so it's not really an issue.

can we have it to use the Makefile targets?

Those are very very similar to others we have elsewhere and to the one we have downstream.

can we have the top repo Dockerfile that generates our upstream images using those konflux Dockerfile instead or at least have the same kind of compilation/flags etc... if we have this upstream i rather don't have multiple ways in the repo to product images...

Well I was thinking the other way. If we get pac upstream, we just have to remove .konflux (and one .tetkon repository) and we are good to go. The idea being to, at least for now, keep our konflux setup segregated from the "upstream" setup.

chmouel commented 3 days ago

can you stash and add a better description of the changes please?

savitaashture commented 1 day ago

can you stash and add a better description of the changes please?

@chmouel updated please take a look Thank you

chmouel commented 1 day ago

I know it's kind of a nitpick but good software hygiene would be to explain what is konflux in the description of the PR and what it does, just adding "Added necessary changes for onboarding PAC to Konflux." like everyone knows what is Konflux is a bit hasted..

I try to make sure for others when making PR to make a great description of what is intended for someone who is doing reviews or for others that follows the project ie: https://github.com/openshift-pipelines/pipelines-as-code/pull/1826 https://github.com/openshift-pipelines/pipelines-as-code/pull/1825. I would like to make sure we have the same for other PRs (except if it's a one line change trivial fix obv)

savitaashture commented 13 hours ago

I know it's kind of a nitpick but good software hygiene would be to explain what is konflux in the description of the PR and what it does, just adding "Added necessary changes for onboarding PAC to Konflux." like everyone knows what is Konflux is a bit hasted..

I try to make sure for others when making PR to make a great description of what is intended for someone who is doing reviews or for others that follows the project ie: #1826 #1825. I would like to make sure we have the same for other PRs (except if it's a one line change trivial fix obv)

Agree @chmouel :+1: Updated now i corrected English with chatgpt

chmouel commented 13 hours ago

sounds good thanks