Add changes for onboarding PAC to Konflux #1795
Conversation
savitaashture
force-pushed
the
pac-konflux-changes
branch
from
4b708bc
to
f2fdb3d
Compare
|
/test linters |
savitaashture
force-pushed
the
pac-konflux-changes
branch
from
62329da
to
637e2b9
Compare
| RUN groupadd -r -g 65532 nonroot && useradd --no-log-init -r -u 65532 -g nonroot nonroot | ||
| USER 65532 | ||
|
|
||
| ENTRYPOINT ["/ko-app/pipelines-as-code-webhook"] |
There was a problem hiding this comment.
those three Docker files are copy and paste to each others, can't be use a ARG to generate multiple image with one ?
There was a problem hiding this comment.
Not sure if buildah has the option ; I hope it does — if it does, we my be able to try that out (but same as commented elsewhere, we need to update our tool)
There was a problem hiding this comment.
actually we can have one dockerfile for all 3 binaries similar to what we have in downstream p12n repo
COPY --from=builder /tmp/pipelines-as-code-controller ${KO_APP}/pipelines-as-code-controller
COPY --from=builder /tmp/pipelines-as-code-watcher ${KO_APP}/pipelines-as-code-watcher
COPY --from=builder /tmp/pipelines-as-code-webhook ${KO_APP}/pipelines-as-code-webhook
and then in entrypoint we can just use
ENTRYPOINT ["/ko-app/pipelines-as-code-controller"]
But the point is @vdemeester the hack tool refer dockerfile path for each component
we might need to change there
openshift/dockerfiles/cli.Dockerfile
Outdated
| ENV GODEBUG="http2server=0" | ||
| RUN go build -mod=vendor -tags disable_gcp -v \ | ||
| -ldflags "-X github.com/openshift-pipelines/pipelines-as-code/pkg/params/version.Version=${TKN_PAC_VERSION}" \ | ||
| -o /tmp/tkn-pac ./cmd/tkn-pac |
There was a problem hiding this comment.
why don't you use the make target instead of using directly your go files?
There was a problem hiding this comment.
I think this is mostly because they are coming from downstream, and look very similar to any other openshift-pipelines component's Dockerfile 🙃 (aka at some point, I was thinking of generating it)
There was a problem hiding this comment.
well if we want to have this upstream you may use the same target as what we have upstream, having multiple way to compile the binaries across the repository is not really good code hygiene
There was a problem hiding this comment.
I see make sense
actually this https://github.com/openshift-pipelines/pipelines-as-code/blob/main/Dockerfile and PR dockerfile almost same but base images are different
@vdemeester can we try with existing dockerfile and lets make components to point out to this existing Dockerfile 🤔
|
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 |
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 👼🏼 |
|
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
force-pushed
the
pac-konflux-changes
branch
from
637e2b9
to
7c2b4ae
Compare
|
@vdemeester as per the discussion moved everything under |
savitaashture
force-pushed
the
pac-konflux-changes
branch
from
7c2b4ae
to
0b3cfd1
Compare
|
some comments are still not addressed:
|
Nope, at least not yet 😓. Those
Those are very very similar to others we have elsewhere and to the one we have downstream.
Well I was thinking the other way. If we get pac upstream, we just have to remove |
|
can you stash and add a better description of the changes please? |
savitaashture
force-pushed
the
pac-konflux-changes
branch
from
0b3cfd1
to
7228d40
Compare
savitaashture
changed the title
@chmouel updated please take a look Thank you |
|
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) |
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 <[email protected]>
savitaashture
force-pushed
the
pac-konflux-changes
branch
from
7228d40
to
f03669d
Compare
Agree @chmouel 👍 |
|
sounds good thanks |
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 [email protected]
Submitter Checklist
📝 Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).
♽ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.
✨ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).
📖 If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.
🧪 While 100% coverage isn't a requirement, we encourage unit tests for any code changes where possible.
🎁 If feasible, please check if an end-to-end test can be added. See README for more details.
🔎 If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).