fix: Use tlsVerify param in PipelineRun template for pack too#3575
fix: Use tlsVerify param in PipelineRun template for pack too#3575creydr wants to merge 2 commits intoknative:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: creydr The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @matejvasek |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3575 +/- ##
==========================================
+ Coverage 56.13% 56.23% +0.09%
==========================================
Files 180 180
Lines 20465 20465
==========================================
+ Hits 11489 11509 +20
+ Misses 7781 7756 -25
- Partials 1195 1200 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| - "-previous-image=$(params.APP_IMAGE)" | ||
| - "-run-image=$(params.RUN_IMAGE)" | ||
| - "$(params.APP_IMAGE)" | ||
| script: | |
There was a problem hiding this comment.
Could we instead of converting this to a script create a new parameter like INSECURE_REGISTRY (passed to create step as a env var) which could be resolved before this template is applied? I think we could have a conditional assignment to this new variable. We can assign some registryInsecure param which gets then assigned here in template to CNB_INSECURE_REGISTRIES and if tls verify is true, we assign empty string, making it a no-op.
Im pretty sure it should work. Looks a bit cleaner to me. Maybe Im wrong 😄 feel free to way in @lkingland @matejvasek
There was a problem hiding this comment.
Yeah, I was thinking about something like that too, but decided initially against it, because I thought we need to do this check anyhow somewhere and in this template I thought it would not "disturb" the other code. But I can do it and then see...
Currently we use the
tlsVerifyparam only in the pipeline arguments for the s2i pipelines. This leads to issues when on remote builds using insecure registries with the pack builder.This PR addresses it and passes the tlsVerify arg to the pack PipelineRun too. When set, it sets the
CNB_INSECURE_REGISTRIESenv var before building the image.