Skip to content

Conversation

@nalind
Copy link
Member

@nalind nalind commented Oct 20, 2025

What type of PR is this?

/kind bug

What this PR does / why we need it:

The HTTP client configurations used by Add() and the logic that we use for pulling down build contexts that are specified using URLs differ in their setup, and the latter doesn't implement the --tls-verify=false flag.

How to verify it

New integration test!

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

This is also a follow-up to #6274.

This moves the function that we use for retrieving things and putting them into temporary directories into its own package, but leaves behind an API-compatible wrapper function.

Does this PR introduce a user-facing change?

Retrieval of build contexts which are specified using HTTPS URLs now respect the value of the `--tls-verify` flag.

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 20, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nalind

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@TomSweeneyRedHat
Copy link
Member

@nalind, did you intend to use the name tmpdir for the new /pkg directory that you're putting the tls code into?

@nalind
Copy link
Member Author

nalind commented Oct 21, 2025

Yeah, it's used by tmpdir.ForURL(), which is meant to replace define.TempDirForURL().

@nalind nalind force-pushed the temp-dir-for-url-pkg branch from 462f4f3 to 20c9435 Compare October 21, 2025 21:02
@nalind
Copy link
Member Author

nalind commented Oct 21, 2025

Moved those bits to a new internal/httpclient package.

@nalind nalind force-pushed the temp-dir-for-url-pkg branch from 20c9435 to dedabe6 Compare October 22, 2025 18:52
@nalind
Copy link
Member Author

nalind commented Oct 22, 2025

Tweaked a bit to expose the proxy setting directly for callers.

Move the generic "download a thing" function to a package that wasn't
originally intended to only include type definitions, but leave a
wrapper in the old location for compatibility's sake.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Make sure that tmpdir.ForURL() and Add() use the same HTTP client
configurations, both for TLS and proxies, so that we apply the same
settings when fetching build contexts that we do for ADD instructions.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the temp-dir-for-url-pkg branch from dedabe6 to dd78138 Compare October 22, 2025 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved kind/bug Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants