feat: add insecure registries configuration for buildpacks buildstrategy

[sgaist]

Changes

This PR adds the insecure registries configuration option for buildpacks buildstrategy.

This is similar to what the buildah and source-to-image BuildStragegy provide and will allow people using local registries or registries behind self-signed certificate to use buildpacks to build their image.

Related Issue

Not a secure solution but partly related to #1835 while waiting for SHIP-0042 implementation.

Type of PR

/kind feature

Submitter Checklist

• Includes tests if functionality changed/was added
• Includes docs if changes are user-facing
• Kind label has been set
• Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

The buildpacks sample build strategies now allow the configuration of insecure registries in a fashion similar to buildah and source-to-image.

[SaschaSchwarze0]

Thanks @sgaist for your contribution. Some ideas/questions/suggestions:

• In a Build, one can set .spec.output.insecure = true. For a strategy-managed push buildstrategy (like buildpacks), we can pass this value to the build strategy using the system-defined parameter shp-output-insecure. Today, we only honor this in BuildKit's sample build strategy: https://github.com/shipwright-io/build/blob/v0.19.2/samples/v1beta1/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml#L69-L70. I think with the CNB variable you are using, we can also support it in Buildpacks with some scripting in the bash script. We should do this, though, it would only be for the output registry. In most scenarios, that is probably enough.
• The parameter that you are added is a single string. Users will need to know the format that Buildpacks expect (like how to separate multiple registry hosts). For consistency, it would be nice to have it as an array like in BuildAh. Do you see blockers for this ?
• Brings me to the question, whether you think this is only needed for the registry the image is pushed to (then I'd say supporting shp-output-insecure is enough), or if you think that users may also want to access other images (only the run-image comes to my mind) from an insecure registry.

[sgaist]

Thanks @sgaist for your contribution. Some ideas/questions/suggestions:

* In a Build, one can set `.spec.output.insecure = true`. For a strategy-managed push buildstrategy (like buildpacks), we can pass this value to the build strategy using the [system-defined parameter `shp-output-insecure`](https://github.com/shipwright-io/build/blob/main/docs/buildstrategies.md#system-parameters). Today, we only honor this in BuildKit's sample build strategy: https://github.com/shipwright-io/build/blob/v0.19.2/samples/v1beta1/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml#L69-L70. I think with the CNB variable you are using, we can also support it in Buildpacks with some scripting in the bash script. We should do this, though, it would only be for the output registry. In most scenarios, that is probably enough.

Do you mean have an export CNB_INSECURE_REGISTRIES line based on shp-output-insecure in the script ?

* The parameter that you are added is a single string. Users will need to know the format that Buildpacks expect (like how to separate multiple registry hosts). For consistency, it would be nice to have it as an array like in BuildAh. Do you see blockers for this ?

My bad, I thought I had updated the description, it's a comma separated list. I will fix while we discuss design.

I wanted to use an array for it however it seems that it's something that is not possible for environment variable content but maybe I missed something during my tests.

* Brings me to the question, whether you think this is only needed for the registry the image is pushed to (then I'd say supporting `shp-output-insecure` is enough), or if you think that users may also want to access other images (only the run-image comes to my mind) from an insecure registry.

I completely missed that second scenario and I think that is also a valid use case.

[SaschaSchwarze0]

@sgaist correct, it means to programmatically fill the content of the CNB_INSECURE_REGISTRIES environment variable depending on how shp-output-insecure is set. That way (= by adding logic to the bash script), you can also support an array like in the other sample strategies that do it for BuildAh.

[sgaist]

@SaschaSchwarze0 There's another thing that could be done to simplify things in these BuildStrategies: pack's lifecycle now also has the creator executable that run the five phases in order.
That would shorten the code.
Since it's not directly related to this change, it could be done in a follow PR. What do you think ?

[SaschaSchwarze0]

@SaschaSchwarze0 There's another thing that could be done to simplify things in these BuildStrategies: pack's lifecycle now also has the creator executable that run the five phases in order. That would shorten the code. Since it's not directly related to this change, it could be done in a follow PR. What do you think ?

We had gone the other way in #990 for reasons related to the process type. Whether creator arguments these days would allow us to go back, I do not know.

[SaschaSchwarze0]

@sgaist you have not permitted maintainers to make changes to your pull request. Can you either allow this, or rebase your branch and cherry-pick SaschaSchwarze0@9cc2c1d. Tyia.

[sgaist]

@SaschaSchwarze0 There's another thing that could be done to simplify things in these BuildStrategies: pack's lifecycle now also has the creator executable that run the five phases in order. That would shorten the code. Since it's not directly related to this change, it could be done in a follow PR. What do you think ?

We had gone the other way in #990 for reasons related to the process type. Whether creator arguments these days would allow us to go back, I do not know.

Thanks for the pointer.
I'll have to do some more checks so it's clearly out of scope for this specific PR.

@sgaist you have not permitted maintainers to make changes to your pull request. Can you either allow this, or rebase your branch and cherry-pick SaschaSchwarze0@9cc2c1d. Tyia.

Done !
I cannot change that setting as it's a PR done from my institute's GitHub organization and not a personal fork but happy to integrate your changes.

[SaschaSchwarze0]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [SaschaSchwarze0]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment