-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add a CRD field to replace InsecureSkipVerify=true #1081
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Are you thinking of passing a boolean parameter for passing in InsecureSkipVerify:true
but defaulting to false
?
Or are you thinking we can just drop it?
I had the same q as @kbrock. Do we need a setting for users that might need to set verify false? |
Checked commit bdunne@485c443 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint **
|
customTransport := http.DefaultTransport.(*http.Transport).Clone() | ||
customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} | ||
customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: !sslVerify} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, but can we also just not set it? Something like
customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: !sslVerify} | |
if !sslVerify { | |
customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: !sslVerify} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging anyway.
This PR has been effectively backported to |
Add a CRD field to replace InsecureSkipVerify=true (cherry picked from commit 5a376e9) # Conflicts: # manageiq-operator/config/crd/bases/manageiq.org_manageiqs.yaml # manageiq-operator/pkg/apis/manageiq/v1alpha1/zz_generated.deepcopy.go
Unclean backport... Trying in #1128 but tests aren't passing yet. |
@bdunne I moved this to morphy/yes? so it wouldn't get in the way of other backports. |
Test image: docker.io/bdunne/manageiq-operator:ssl_verifyAdd optional CR value
OIDCOAuthIntrospectionSSLVerify
to determine whether or not to verify SSL when fetching the introspection URL. Default to SSLVerify for safety.