Skip to content

Conversation

@bparees
Copy link
Contributor

@bparees bparees commented Sep 11, 2020

as far as i know this works (it's why we allow you to configure proxy CAs). https proxies do not work with builds for purposes of source retrieval, due to a limitation in the git version we use. That limitation will be going away in 4.6. It's possible that restriction is where this line came from, i'm not sure.

So maybe it's worth noting that restriction in the build docs explicitly, or we just let it go knowing that the problem is going away sooner than later anyway.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 11, 2020
@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2020

cc @adambkaplan @danehans @wking

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2020

@wking i left If this is not specified, then httpProxy is used for both HTTP and HTTPS connections. for now since it sounds like the CVO is going to be fixed to comply w/ this claim and i'm not aware of another component that isn't compliant, but i still don't feel great about it.

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2020

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2020

@openshift/team-documentation ptal

@bobfuru
Copy link
Contributor

bobfuru commented Sep 12, 2020

Cc: @bergerhoffer for review. It looks like this was content added in #16635 for OCP 4.2.

@bergerhoffer
Copy link
Contributor

@bparees so are you saying that the text The URL scheme must be http; https is currently not supported. is true for 4.3-4.5, but that restriction is no longer true for 4.6+?

@bergerhoffer
Copy link
Contributor

@danehans Can you confirm this as well? The request to add this note was here: #16635 (comment)

@bparees
Copy link
Contributor Author

bparees commented Sep 14, 2020

@bparees so are you saying that the text The URL scheme must be http; https is currently not supported. is true for 4.3-4.5, but that restriction is no longer true for 4.6+?

I don't think it was ever restricted, even 4.2 tolerates both:
https://github.com/openshift/cluster-network-operator/blob/release-4.2/pkg/controller/proxyconfig/validation.go#L55

@danehans
Copy link
Contributor

@bergerhoffer I have no objections as long as https://issues.redhat.com/browse/BUILD-68 is completed.

@bergerhoffer
Copy link
Contributor

Got ack from @danehans that it's okay to remove this, as long as there is a note about the lack of git clone support when using an https scheme. This is already noted as a known issue the release notes: "Git clone operations that go through an HTTPS proxy fail. Non-TLS (HTTP) proxies can be used successfully. (BZ#1750650)"

Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

This LGTM

@bergerhoffer
Copy link
Contributor

@gpei or @lihongan - can you please review this? Thanks!

@gpei
Copy link

gpei commented Sep 23, 2020

The https proxy works well for installer side. @wzheng1 could you please help to provide an ack on this? Considering it's the restriction on BUILD.

@wzheng1
Copy link

wzheng1 commented Sep 24, 2020

LGTM, thanks!

@bparees
Copy link
Contributor Author

bparees commented Oct 6, 2020

@bergerhoffer can we merge this?

@jboxman
Copy link
Contributor

jboxman commented Oct 6, 2020

@bparees did I trip over this by accident?

#26084

Or is this unrelated to https://bugzilla.redhat.com/show_bug.cgi?id=1845929? I don't want to collide if my PR is superseded by this.

Thanks!

@bparees
Copy link
Contributor Author

bparees commented Oct 6, 2020

Or is this unrelated to https://bugzilla.redhat.com/show_bug.cgi?id=1845929?

unrelated.

@bergerhoffer
Copy link
Contributor

@bparees Sorry missed the notification. Merging!

@bergerhoffer bergerhoffer merged commit 3d854ec into openshift:master Oct 6, 2020
@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.6

@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.5

@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.4

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #26157

Details

In response to this:

/cherrypick enterprise-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.3

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #26158

Details

In response to this:

/cherrypick enterprise-4.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #26159

Details

In response to this:

/cherrypick enterprise-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #26160

Details

In response to this:

/cherrypick enterprise-4.3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

Labels

branch/enterprise-4.3 branch/enterprise-4.4 branch/enterprise-4.5 branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants