Skip to content

expose oauth config in config#1426

Merged
openshift-bot merged 2 commits intoopenshift:masterfrom
deads2k:deads-expose-auth-config
Mar 31, 2015
Merged

expose oauth config in config#1426
openshift-bot merged 2 commits intoopenshift:masterfrom
deads2k:deads-expose-auth-config

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Mar 24, 2015

Exposes the oauth config. I'll add integration tests once we're able to modify the actual config.

Here is an example of how it looks:
https://gist.github.com/deads2k/da5618636ce4bf83c0c2

@liggitt

@deads2k deads2k force-pushed the deads-expose-auth-config branch from 9051eec to 1448e42 Compare March 25, 2015 20:32
liggitt
liggitt reviewed Mar 26, 2015
View reviewed changes
pkg/cmd/server/api/types.go Outdated
Copy link
Contributor

@liggitt liggitt Mar 26, 2015

Choose a reason for hiding this comment

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

RequestHeaderIdentityProvider?

@deads2k deads2k force-pushed the deads-expose-auth-config branch from 1448e42 to abd71e1 Compare March 26, 2015 12:01
@deads2k
Copy link
Contributor Author

deads2k commented Mar 26, 2015

[test]

@deads2k
Copy link
Contributor Author

deads2k commented Mar 26, 2015

Comments addressed.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 26, 2015

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1553/)

@liggitt
Copy link
Contributor

liggitt commented Mar 27, 2015

My kingdom for polymorphism

@liggitt
Copy link
Contributor

liggitt commented Mar 27, 2015

Ok, I'm pretty happy with this. @deads2k can you add examples of the different identity provider stanzas? @brenton, this would mean that if you want anything other than the default anypassword identity provider, you need to run from config. What's the best time/way for us to coordinate moving the RPM-installed version to run from config files?

@liggitt
Copy link
Contributor

liggitt commented Mar 27, 2015

I also opened PR deads2k#8 against this branch to warn about deprecating the envvar method of configuring identity providers, and honor (for now) an envvar to deny login

@deads2k deads2k force-pushed the deads-expose-auth-config branch 3 times, most recently from 71c4138 to a1e754b Compare March 30, 2015 20:20
@liggitt liggitt mentioned this pull request Mar 31, 2015
Closed
@deads2k deads2k force-pushed the deads-expose-auth-config branch from a1e754b to 2ec800e Compare March 31, 2015 12:54
@deads2k
Copy link
Contributor Author

deads2k commented Mar 31, 2015

rebased.

@deads2k
Copy link
Contributor Author

deads2k commented Mar 31, 2015

@sdodson @detiber This pull forces users to use a config file to use any identity provider other than "allow all". It's a direction that makes sense for understandability of config and preventing mistakes, but it may affect the ansible config. Any concerns before we merge?

liggitt
liggitt reviewed Mar 31, 2015
View reviewed changes
pkg/cmd/server/api/types.go
Copy link
Contributor

@liggitt liggitt Mar 31, 2015

Choose a reason for hiding this comment

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

open a followup issue for me to make this structured. secrets have to be in pairs (one for signing, one for encryption, with specific length requirements on the encryption key)

@sdodson
Copy link
Member

sdodson commented Mar 31, 2015

@deads2k This is user centric or do other parts use this? WRT to users, will osc login do the right thing so that users won't know that anything has changed?

@liggitt
Copy link
Contributor

liggitt commented Mar 31, 2015

@sdodson no end-user impact, this is purely a cluster-admin config facing

@deads2k deads2k force-pushed the deads-expose-auth-config branch from dd9b4b2 to 0b6e0c8 Compare March 31, 2015 17:01
@liggitt
Copy link
Contributor

liggitt commented Mar 31, 2015

rebase, then LGTM

@deads2k deads2k force-pushed the deads-expose-auth-config branch from 0b6e0c8 to be39e07 Compare March 31, 2015 18:13
@deads2k
Copy link
Contributor Author

deads2k commented Mar 31, 2015

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 31, 2015

Evaluated for origin up to be39e07

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 31, 2015

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1363/) (Image: devenv-fedora_1177)

openshift-bot pushed a commit that referenced this pull request Mar 31, 2015
Merge pull request #1426 from deads2k/deads-expose-auth-config
f6c4feb 
Merged by openshift-bot
@openshift-bot openshift-bot merged commit f6c4feb into openshift:master Mar 31, 2015
@deads2k deads2k deleted the deads-expose-auth-config branch March 31, 2015 18:52
@liggitt liggitt mentioned this pull request Apr 11, 2015
Closed
@coveralls
Copy link

coveralls commented Nov 14, 2016

Coverage Status

Changes Unknown when pulling be39e07 on deads2k:deads-expose-auth-config into * on openshift:master*.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@deads2k @openshift-bot @liggitt @sdodson @coveralls