Skip to content

docs: document proxy support #414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Nov 19, 2024
Merged

docs: document proxy support #414

merged 14 commits into from
Nov 19, 2024

Conversation

SasSwart
Copy link
Contributor

This PR closes #411

@SasSwart SasSwart marked this pull request as ready for review November 10, 2024 21:18
@SasSwart
Copy link
Contributor Author

SasSwart commented Nov 10, 2024

Still thinking about an integration test for this, to ensure that it always works. I'm happy to do that as a second PR. Keen to hear from reviewers though.

Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Just a couple of nits. I especially like the demonstration you provided, it really aids understanding.

@johnstcn
Copy link
Member

Still thinking about an integration test for this, to ensure that it always works. I'm happy to do that as a second PR. Keen to hear from reviewers though.

Separate PR is fine 👍 but always a fan of an integration test for this sort of functionality.

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this documentation! There are a few things that I found that I think can be improved, but there's two thoughts I'd like to leave you with that I think can be beneficial to your writing in the future:

  • Can this be said in less words, i.e. is this critical to get the point across?
  • Can this be made more concrete?

SasSwart and others added 7 commits November 11, 2024 13:39
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
@mafredri mafredri changed the title chore(docs) document proxy support chore(docs): document proxy support Nov 11, 2024
@mafredri mafredri changed the title chore(docs): document proxy support docs: document proxy support Nov 11, 2024
@SasSwart
Copy link
Contributor Author

@mafredri I've applied all of your review notes. I'm going ahead and merging this. If you would like to provide further feedback, I will open another PR to update :)

Thanks for the review.

@SasSwart SasSwart enabled auto-merge (squash) November 19, 2024 13:24
@SasSwart SasSwart merged commit 621512c into main Nov 19, 2024
4 checks passed
@SasSwart SasSwart deleted the jjs/411 branch November 19, 2024 13:28
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on the rewrite, looks good. Spotted a stray deletion without compensation tho 😄

As before, this command yields a shell inside an Envbuilder built environment. Feel free to test it and then exit the container. Assuming this worked, Envbuilder will have cloned a repository and built the relevant container using a proxy that required accepting a custom CA certificate.

### Bonus
To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image.
To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there will be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hotfixing

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.

chore: document and test HTTP_PROXY support
3 participants