Skip to content

Conversation

@mayooot
Copy link

@mayooot mayooot commented Aug 22, 2025

fix: #466

@mayooot
Copy link
Author

mayooot commented Aug 22, 2025

@jglogan Could you please take a look?

@egernst
Copy link
Contributor

egernst commented Aug 27, 2025

@mayooot - thanks for the PR.

From reading the original issue discussion, I think the direction suggested was to add the utility in github.com/apple/containerization since we'll also want to address this there, and then use that utility in container?

@mayooot
Copy link
Author

mayooot commented Aug 27, 2025

@mayooot - thanks for the PR.

From reading the original issue discussion, I think the direction suggested was to add the utility in github.com/apple/containerization since we'll also want to address this there, and then use that utility in container?

Ok, thanks for your reply. Do you have any suggestions on where to place this utility?

@jglogan
Copy link
Contributor

jglogan commented Sep 5, 2025

Do you have any suggestions on where to place this utility?

@mayooot The issue in the containerization repo is apple/containerization#255. Having it there could allow us to do the right thing for image downloads as well as kernel.

I'd start by having a look at the issue (see the comment at apple/containerization#255 (comment)) and then adapting what's in your PR to that. IMO ContainerizationExtras is the best place to put it the proxy utility.

Then this PR would just contain your kernel download fix. I'll create another issue for the image pull/push fix in container so that we can keep this PR small and quick to merge.

@jglogan
Copy link
Contributor

jglogan commented Sep 5, 2025

Added #578 for image push/pull.

@mayooot
Copy link
Author

mayooot commented Sep 5, 2025

Do you have any suggestions on where to place this utility?

@mayooot The issue in the containerization repo is apple/containerization#255. Having it there could allow us to do the right thing for image downloads as well as kernel.

I'd start by having a look at the issue (see the comment at apple/containerization#255 (comment)) and then adapting what's in your PR to that. IMO ContainerizationExtras is the best place to put it the proxy utility.

Then this PR would just contain your kernel download fix. I'll create another issue for the image pull/push fix in container so that we can keep this PR small and quick to merge.

Thanks a lot, I'll check it out ASAP.

@jglogan
Copy link
Contributor

jglogan commented Sep 5, 2025

For this PR, this will probably need to be fixed:

launchctl setenv HTTP_PROXY $HTTP_PROXY

Might even be good to add a little fuzzing in the git workflow so that it randomly chooses HTTPS_PROXY or https_proxy.

@jglogan jglogan mentioned this pull request Sep 12, 2025
7 tasks
@jglogan
Copy link
Contributor

jglogan commented Sep 12, 2025

@mayooot Have a go at an updated version once we merge #606. You should be able to bring in your proxy utility then.

Did you want to have a crack at apple/containerization#255 and then the container updates to use the affected image store APIs?

jglogan added a commit that referenced this pull request Sep 12, 2025
- Makes available the proxy utility from containerization#288.

## Type of Change
- [ ] Bug fix
- [x] New feature  
- [ ] Breaking change
- [ ] Documentation update

## Motivation and Context
The proxy utility allows forward progress on #533.

## Testing
- [x] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
@mayooot
Copy link
Author

mayooot commented Sep 13, 2025

@mayooot Have a go at an updated version once we merge #606. You should be able to bring in your proxy utility then.

Did you want to have a crack at apple/containerization#255 and then the container updates to use the affected image store APIs?

Sure, I can take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Kernel download honors HTTP_PROXY but not other standard environment variables.

3 participants