Skip to content

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Feb 19, 2015

Removes cluster-admin rights from system:authenticated and system:unauthenticated.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 19, 2015

[test]

@openshift-bot
Copy link
Contributor

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

@jwforres jwforres added this to the 0.4.0 (beta2) milestone Feb 19, 2015
@deads2k deads2k force-pushed the deads-the-big-one branch 2 times, most recently from 75a7967 to 2c64385 Compare February 19, 2015 18:23
@deads2k deads2k changed the title [waiting] tighten authorization rules tighten authorization rules Feb 19, 2015
@deads2k
Copy link
Contributor Author

deads2k commented Feb 19, 2015

@smarterclayton @liggitt fish or cut bait.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 19, 2015

2/3 on travis.

@smarterclayton
Copy link
Contributor

So what README and documentation plans do you have changed so that once this happens every openshift user who tries it isn't broke?

@deads2k
Copy link
Contributor Author

deads2k commented Feb 19, 2015

Currently, the readme has people using the cluster-admin id, so anyone following those steps continues to work. We already document how to add users to roles, those commands haven't changed.

Is there another sample besides sample-app that people are using?

I probably should write up documentation for openshift ex new-project so that people know they don't have to follow separate steps.

@liggitt
Copy link
Contributor

liggitt commented Feb 19, 2015

Should update the help shown in the web UI when no projects exist to tell them to run new-project --admin <current user>

@deads2k
Copy link
Contributor Author

deads2k commented Feb 19, 2015

Should update the help shown in the web UI when no projects exist to tell them to run new-project --admin

Ok. I guess I'll also craft something for the mailing list and maybe add instructions to set up a user capable of viewing default for the sample app

@derekwaynecarr
Copy link
Member

Doesnt the sample app use 'test'?

@deads2k
Copy link
Contributor Author

deads2k commented Feb 20, 2015

It uses both. default for the docker registry and test for the rest.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 20, 2015

@liggitt I updated the readme with a little more detail. Care to make sure it works for you?

@smarterclayton
Copy link
Contributor

Not going to merge this until we cut another release image (v0.3.2)

@deads2k
Copy link
Contributor Author

deads2k commented Feb 24, 2015

Rebased. Removed the readme updates since they were superceded by #1104 which can go in separately.

@smarterclayton
Copy link
Contributor

Does everyone know this is coming and how to react?

@deads2k
Copy link
Contributor Author

deads2k commented Feb 24, 2015

I'll send out a note now.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 24, 2015

Does everyone know this is coming and how to react?

Yes, everyone knows this is coming. Basic instructions were provided in an email and link to our more complete documentation was also provided. I also included information about gathering policy and bindings for bug reports (not that I think anything could possibly go wrong :) )

@liggitt
Copy link
Contributor

liggitt commented Feb 24, 2015

just noticed the Jenkins example isn't using identity yet (there's a bug open for cert validation errors, but identity would hit it also). Wondering if we should wait until we have a good way to inject a kubeconfig as a secret.

@smarterclayton
Copy link
Contributor

Up to Ben

On Feb 24, 2015, at 9:14 AM, Jordan Liggitt [email protected] wrote:

just noticed the Jenkins example isn't using identity yet (there's a bug open for cert validation errors, but identity would hit it also). Wondering if we should wait until we have a good way to inject a kubeconfig as a secret.


Reply to this email directly or view it on GitHub.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 25, 2015

@bparees see above.

@deads2k deads2k changed the title tighten authorization rules [DO NOT MERGE] tighten authorization rules Feb 25, 2015
@deads2k
Copy link
Contributor Author

deads2k commented Feb 25, 2015

Bug https://bugzilla.redhat.com/show_bug.cgi?id=1196022 found.

Do not merge this until we get #1140

@bparees
Copy link
Contributor

bparees commented Feb 25, 2015

Not only do the jenkins setup steps need to be fixed to use identity, the jenkins job itself would need to have the certs (because it runs osc commands too) which it currently does not, so there's significant rework needed. (or use the --insecure flag, anyway, after doing an osc login or something).

I haven't had a lot of people asking me questions about the jenkins example so my guess is it's not getting a lot of attention, but it would be nice to keep it working.

@deads2k
Copy link
Contributor Author

deads2k commented Feb 25, 2015

but it would be nice to keep it working.

It's currently broken, so it would be "make it work again".

@bparees
Copy link
Contributor

bparees commented Feb 25, 2015

@deads2k is that the sound of volunteering i hear?

@smarterclayton
Copy link
Contributor

It's definitely something people are looking at. Let's create a maint card if it still needs work after David's change.

On Feb 25, 2015, at 7:25 AM, Ben Parees [email protected] wrote:

Not only do the jenkins setup steps need to be fixed to use identity, the jenkins job itself would need to have the certs (because it runs osc commands too) which it currently does not, so there's significant rework needed. (or use the --insecure flag, anyway, after doing an osc login or something).

I haven't had a lot of people asking me questions about the jenkins example so my guess is it's not getting a lot of attention, but it would be nice to keep it working.


Reply to this email directly or view it on GitHub.

@bparees
Copy link
Contributor

bparees commented Feb 26, 2015

I have a pr in progress to get it working again. That's where I hit the duplicate tag imagerepo issue.

Ben Parees | OpenShift

-----Original Message-----
From: Clayton Coleman [[email protected]]
Received: Thursday, 26 Feb 2015, 10:06
To: openshift/origin [[email protected]]
CC: Ben Parees [[email protected]]
Subject: Re: [origin] [DO NOT MERGE] tighten authorization rules (#1074)

It's definitely something people are looking at. Let's create a maint card if it still needs work after David's change.

On Feb 25, 2015, at 7:25 AM, Ben Parees [email protected] wrote:

Not only do the jenkins setup steps need to be fixed to use identity, the jenkins job itself would need to have the certs (because it runs osc commands too) which it currently does not, so there's significant rework needed. (or use the --insecure flag, anyway, after doing an osc login or something).

I haven't had a lot of people asking me questions about the jenkins example so my guess is it's not getting a lot of attention, but it would be nice to keep it working.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub:
#1074 (comment)

@deads2k deads2k changed the title [DO NOT MERGE] tighten authorization rules tighten authorization rules Feb 26, 2015
@liggitt
Copy link
Contributor

liggitt commented Mar 2, 2015

reminder to myself to make sure we have the jenkins example working with secrets before merging this

@brenton
Copy link
Contributor

brenton commented Mar 3, 2015

@deads2k, @bparees, @liggitt, Is this realistic for beta2 at this point? Some of the other things Erik is planning to demo need this.

@liggitt
Copy link
Contributor

liggitt commented Mar 3, 2015

if we tighten this, the core path (master/node/registry/router) works, but other things like the Jenkins example will stop working until we can get identity info into it. We're planning to do that with secrets, but those won't be in origin until the rebase merges, and then there is some follow-up work to make creating a secret for Jenkins reasonable for an end-user.

tl;dr - this can go in now if we accept things like the Jenkins example breaking until we get secret support into origin.

@smarterclayton
Copy link
Contributor

That's fine with me

On Mar 3, 2015, at 10:06 AM, Jordan Liggitt [email protected] wrote:

if we tighten this, the core path (master/node/registry/router) works, but other things like the Jenkins example will stop working until we can get identity info into it. We're planning to do that with secrets, but those won't be in origin until the rebase merges, and then there is some follow-up work to make creating a secret for Jenkins to use reasonable for an end-user.

tl;dr - this can go in now if we accept things like the Jenkins example breaking until we get secret support into origin.


Reply to this email directly or view it on GitHub.

@brenton
Copy link
Contributor

brenton commented Mar 3, 2015

I saw the menion about jenkins in the comment. The beta training isn't making use of jenkins right now so I think we'd be OK. @thoraxe (let us know if you have concerns)

@thoraxe
Copy link
Contributor

thoraxe commented Mar 3, 2015

I'm fine with Jenkins not working for beta2.

@liggitt
Copy link
Contributor

liggitt commented Mar 3, 2015

pull the trigger! [merge]

@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

Evaluated for origin up to 65d868f

openshift-bot pushed a commit that referenced this pull request Mar 3, 2015
@openshift-bot openshift-bot merged commit 2c89f60 into openshift:master Mar 3, 2015
@deads2k deads2k deleted the deads-the-big-one branch March 3, 2015 18:14
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Aug 17, 2017
…service-catalog/' changes from 8f07b7b..7e650e7

7e650e7 origin build: add origin tooling
f32eec2 unit test for ./pkg/rest/core/fake rest client, addresses openshift#860 Test ready to be reviewed (openshift#1113)
e388aee explicitly always prefer latest OSBAPI Version (openshift#1138)
962429e Merge branch 'pr/1135'
b6ee7ef fix rbac
cb1beb9 Merge branch 'pr/1131'
ecc5c01 Update Code of Conduct (openshift#1137)
e7c5ab3 address one more PR comment
ddcbbad address PR comments
652a83b fix test expectation to match the new error message for missing service class
33417cc address PR comments
565fccf Use the chart name instead of the namespace (openshift#1102)
bc61919 Add new terminal failure binding condition (openshift#1057)
4e642d5 Added more detailed instructions on how to setup the repo (openshift#1114)
bdaea23 update unit tests (openshift#1123)
88a9642 validate the apiserver options (openshift#1116)
b0af5fc fix whitespace in the copyright section
dee796a generated type changes
ef585c4 Rename the directory from default to defaultservicename to conform to go style guide. Wire admission controller into the apiserver
0b5d6c6 add firewall troubleshooting section (openshift#1040)
fd9e6bc Fix Typo in Events Code of Conduct (openshift#1126)
ebe6506 Fix Typo in Terminology (openshift#1128)
0038b1e Merge branch 'pr/1122'
8411f31 make deprovisioning an instance asynchronously not fall-through to synchronous deprovision (openshift#1067)
76c1d93 handle failures from list and test the not ready condition, cleanup
9241296 finish unit tests, passing
ed75774 Minor fixes based on go report card
9911e8d Add GoReport Widget (openshift#1121)
dd24e5c clean up old cruft
08276c6 generated file changes
6489d90 Implement the default plan in admission controller
a6bb576 Code: Instance/Binding parameters from secret (openshift#1079)
10bb148 Update generated files (openshift#1115)
5291e6f v0.0.15 (openshift#1118)
28a1ea6 Merge branch 'pr/1104'
bb4a2d2 Merge branch 'pr/1097'
1c14a90 push all arch images on release tags (openshift#1108)
b587b2c Improve log output for deprovision
8887561 Remove PodPreset embedding from Binding (openshift#1030)
1abdcc8 Adjust helm/tiller installation instructions (openshift#1091)
f636f99 only skip tls verify if not behind the aggregator (openshift#1101)
43b40ab controller_broker unit test bullet-proofing openshift#1077 (openshift#1099)
bb596b8 Use data store instead of database (openshift#1100)
04fa477 Implementation: Support for Bearer token auth between Service Catalog and brokers (openshift#1053)
9e46d3c refactor Jenkins e2e tests (openshift#1082)
1f0a41e remove old/misleading comments about only doing soft delete if it's "our turn--" i.e. only if the finalizer we care about is at the head of the finalizers list.
5c1d9b8 Update OSB client (openshift#1085)
a6e80ea Only do work for instances from a single queue (openshift#1074)
2bd85d6 Merge branch 'pr/1076'
e324287 Tweaks to the walkthrough for local-up-cluster
d8b7899 Add a note to the walkthrough about getting bindings when using the aggregator (openshift#1078)
ea44cf1 msg on Environment Variables to set for e2e (openshift#1070)
d15554a Merge branch 'pr/1017'
faf966e Add comment re: async race condition in integration tests
ed2e096 v0.0.14 (openshift#1071)
fc84ffd more PR feedback
283bed4 Add integration tests and some error checking; PR feedback
903a7a7 Add terminal condition for instance and do not retry failed provisions
REVERT: 8f07b7b origin: add required patches

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: 7e650e7e39c3fc79a8ecc061cce2a70e899406ff
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
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.

9 participants