Skip to content
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

Bug 157608, Removed filename key and value #9138

Merged
merged 1 commit into from
May 11, 2018

Conversation

ahardin-rh
Copy link
Contributor

@ahardin-rh ahardin-rh added this to the Future Release milestone May 9, 2018
@ahardin-rh ahardin-rh self-assigned this May 9, 2018
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 9, 2018
@ahardin-rh
Copy link
Contributor Author

@cevich PTAL. Is this change for 3.10 only, or should it also be applied to 3.9? Thanks!

@cevich
Copy link

cevich commented May 10, 2018

@ahardin-rh IDK 😞 I only tested 3.10, not sure when that option was removed. @sdodson ?

@ahardin-rh
Copy link
Contributor Author

@jianlinliu Please provide a QE review

@@ -1936,7 +1936,7 @@ openshift_deployment_type=origin
endif::[]

# uncomment the following to enable htpasswd authentication; defaults to DenyAllPasswordIdentityProvider
#openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider', 'filename': '/etc/origin/master/htpasswd'}]
#openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider', '/etc/origin/master/htpasswd'}]

Copy link

Choose a reason for hiding this comment

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

This is a JSON key, so the value's gotta be clobbered as well:

#openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider'}]

Copy link

Choose a reason for hiding this comment

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

same fore below.

@@ -1936,7 +1936,7 @@ openshift_deployment_type=origin
endif::[]

# uncomment the following to enable htpasswd authentication; defaults to DenyAllPasswordIdentityProvider
#openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider', 'filename': '/etc/origin/master/htpasswd'}]
#openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider', '/etc/origin/master/htpasswd'}]

Choose a reason for hiding this comment

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

Remove string '/etc/origin/master/htpasswd'

The path is hard-coded now.

Copy link

Choose a reason for hiding this comment

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

Ya, that's what I was trying to say also 😄

@cevich
Copy link

cevich commented May 10, 2018

@michaelgugino off-hand, do you know when that value was hard-coded? Was it for 3.9 also?

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 10, 2018
@ahardin-rh
Copy link
Contributor Author

@cevich @michaelgugino Thanks! I think I got them all now 🍻

@cevich
Copy link

cevich commented May 10, 2018

LGTM (FWIW)

@ahardin-rh
Copy link
Contributor Author

Looks like this is for 3.10+ https://bugzilla.redhat.com/show_bug.cgi?id=1576088#c4

@cevich
Copy link

cevich commented May 10, 2018

@michaelgugino off-hand,

nvm...question answered. It's 3.10 and beyond only.

@ahardin-rh
Copy link
Contributor Author

QE approved in the BZ; no peer review needed

@ahardin-rh ahardin-rh merged commit 4d2c0a8 into openshift:master May 11, 2018
@ahardin-rh
Copy link
Contributor Author

/cherrypick enterprise-3.10

@openshift-cherrypick-robot

@ahardin-rh: new pull request created: #9212

In response to this:

/cherrypick enterprise-3.10

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.

@ahardin-rh ahardin-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-done Signifies that the peer review team has reviewed this PR labels May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-3.10 size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants