OpenShift auth handler for v2 docker registry #1472

pravisankar · 2015-03-26T19:06:10Z

No description provided.

ncdc · 2015-03-26T19:12:16Z

pkg/dockerregistry/auth/openshift.go

+}
+
+// Status returns the HTTP Response Status Code for this authChallenge.
+func (ac *authChallenge) Status() int {


I would remove this method and just return http.StatusUnauthorized directly in ServeHTTP

pravisankar · 2015-04-01T21:55:11Z

@ncdc tested the code (docker push/pull is working as expected)
I'll update the test cases, please take a look

pravisankar · 2015-04-03T00:18:39Z

@ncdc updated, please review

ncdc · 2015-04-03T00:57:18Z

Looks like you might need to run gofmt on test/integration/v2_docker_registry_test.go, according to travis.

ncdc · 2015-04-07T15:46:43Z

pkg/dockerregistry/auth/openshift.go

+
+var _ registryauth.Challenge = &authChallenge{}
+
+type OpenShiftAccess struct {


I'm not sure I like this being a struct. Could we remove it and modify VerifyOpenShiftAccess to take in the 4 fields as arguments instead?

pravisankar · 2015-04-07T20:52:41Z

@ncdc updated as per your feedback, please review

ncdc · 2015-04-07T21:00:47Z

pkg/dockerregistry/auth/openshift_test.go

+		},
+	}
+	for _, item := range table {
+		fmt.Println("Processing item: %s", item)


Don't use println. t.Logf is ok

ncdc · 2015-04-07T21:02:14Z

pkg/dockerregistry/auth/openshift_test.go

+			expectedError:       nil,
+		},
+	}
+	for _, item := range table {


rename item to test

pravisankar · 2015-04-07T22:00:57Z

@ncdc PTAL

ncdc · 2015-04-08T09:55:21Z

[test]

Sent from my iPhone

On Apr 7, 2015, at 6:00 PM, Ravi Sankar Penta [email protected] wrote:

@ncdc PTAL

—
Reply to this email directly or view it on GitHub.

openshift-bot · 2015-04-08T09:55:39Z

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

ncdc · 2015-04-08T11:04:00Z

[test]

ncdc · 2015-04-08T12:02:29Z

[test]

Sent from my iPhone

On Apr 8, 2015, at 7:05 AM, OpenShift Bot [email protected] wrote:

Evaluated for origin up to e384ee7

—
Reply to this email directly or view it on GitHub.

ncdc · 2015-04-08T13:59:53Z

LGTM [merge]

openshift-bot · 2015-04-08T14:00:41Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1737/) (Image: devenv-fedora_1235)

ncdc · 2015-04-08T21:11:59Z

exec flake, [merge]

openshift-bot · 2015-04-08T21:15:41Z

Evaluated for origin up to e384ee7

Merged by openshift-bot

brenton · 2015-04-09T12:20:19Z

Is there a link to a story or a short description somewhere of what this PR enables? I'm guessing this will start enforcing who can push/pull images to the registry.

brenton · 2015-04-09T12:21:40Z

I see the policy is being updated to have a 'images' group.

ncdc · 2015-04-09T13:22:26Z

@brenton the card is here: https://trello.com/c/IdJ6y0Wt/295-5-registry-authentication-authorization-imageregistry-codefreeze

A user will be able to push to an image repository in the OpenShift registry if they have edit or admin rights to a project. For pull, view/edit/admin rights.

Once Docker 1.6 is out and we've switching to using the v2 registry, we'll be securing access to /images and /imageStreamMappings so that only the v2 registry may access those routes. Users will be allowed to access /imageStreams, /imageStreamTags, and /imageStreamImages. See #1652

brenton · 2015-04-09T13:48:45Z

Thanks Andy. This is perfect.

…service-catalog/' changes from 892b0368f0..3064247d05 3064247d05 origin build: add origin tooling 48ecff1 Chart changes for 0.1.2 (openshift#1527) 8727247 Fix change validator to look up serviceclass properly (openshift#1518) b0b138b Broker Reconciliation occurs too frequently (openshift#1514) cc816eb When a SI is unbindable, mark the binding request failed. (openshift#1522) 41290e9 Clarify semantics around RelistDuration (openshift#1516) 6bcb593 Send Context with UpdateInstanceRequest (openshift#1517) c3b72d8 Enforce stricly increasing broker relistRequests (openshift#1515) 01d81e5 Follow up from openshift#1450 and openshift#1444 (openshift#1504) 5e77882 Retry failed deprovision requests (openshift#1505) f6c891d Allow updates of instances that failed a previous update. (openshift#1502) 5107086 Use Plan ID from ExternalProperties when deprovisioning instance (openshift#1501) d897c60 Adding message builder for expected event strings (openshift#1465) 9f6152e update osb client and freeze gorilla context (openshift#1496) c63277e Add unit tests verifying deleting a resource with an on-going operation or in orphan mitigation. (openshift#1490) 4ecca16 Pretty logging for controller_instance.go (openshift#1472) c232db4 Register qemu statics when building non-amd64 images (openshift#1494) 0e54c57 fix 'Spring Cloud Services -> Configuring with Vault' link in docs (openshift#1495) REVERT: 892b0368f0 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 3064247d0544aa43f976d93caea0a11771434ef7

* Working on controller instance. * Finished working controller_instance.go.

ncdc reviewed Mar 26, 2015
View reviewed changes

pravisankar force-pushed the v2registry-changes branch from 3e4df76 to d02d9d6 Compare April 1, 2015 21:49

pravisankar changed the title ~~[DO_NOT_MERGE] OpenShift auth handler for v2 docker registry~~ OpenShift auth handler for v2 docker registry Apr 1, 2015

pravisankar force-pushed the v2registry-changes branch 2 times, most recently from 5bdf21d to a0deb83 Compare April 3, 2015 00:14

pravisankar force-pushed the v2registry-changes branch from a0deb83 to 05edc87 Compare April 6, 2015 17:44

ncdc reviewed Apr 7, 2015
View reviewed changes

UPSTREAM: add context to ManifestService methods

0201c61

pravisankar force-pushed the v2registry-changes branch from 05edc87 to f39ef9c Compare April 7, 2015 20:50

ncdc reviewed Apr 7, 2015
View reviewed changes

pkg/dockerregistry/auth/openshift_test.go

expectedError: nil,

},

}

for _, item := range table {

Copy link

Contributor

ncdc Apr 7, 2015

Choose a reason for hiding this comment

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

rename item to test

OpenShift auth handler for v2 docker registry

e384ee7

pravisankar force-pushed the v2registry-changes branch from f39ef9c to e384ee7 Compare April 7, 2015 22:00

openshift-bot pushed a commit that referenced this pull request Apr 8, 2015

Merge pull request #1472 from pravisankar/v2registry-changes

33f4e20

Merged by openshift-bot

openshift-bot merged commit 33f4e20 into openshift:master Apr 8, 2015

jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018

Pretty logging for controller_instance.go (openshift#1472)

4ecca16

* Working on controller instance. * Finished working controller_instance.go.


		var _ registryauth.Challenge = &authChallenge{}

		type OpenShiftAccess struct {

OpenShift auth handler for v2 docker registry #1472

OpenShift auth handler for v2 docker registry #1472

Uh oh!

Conversation

pravisankar commented Mar 26, 2015

Uh oh!

ncdc Mar 26, 2015

Choose a reason for hiding this comment

Uh oh!

pravisankar commented Apr 1, 2015

Uh oh!

pravisankar commented Apr 3, 2015

Uh oh!

ncdc commented Apr 3, 2015

Uh oh!

ncdc Apr 7, 2015

Choose a reason for hiding this comment

Uh oh!

pravisankar commented Apr 7, 2015

Uh oh!

ncdc Apr 7, 2015

Choose a reason for hiding this comment

Uh oh!

ncdc Apr 7, 2015

Choose a reason for hiding this comment

Uh oh!

pravisankar commented Apr 7, 2015

Uh oh!

ncdc commented Apr 8, 2015

Uh oh!

openshift-bot commented Apr 8, 2015

Uh oh!

ncdc commented Apr 8, 2015

Uh oh!

ncdc commented Apr 8, 2015

Uh oh!

ncdc commented Apr 8, 2015

Uh oh!

openshift-bot commented Apr 8, 2015

Uh oh!

ncdc commented Apr 8, 2015

Uh oh!

openshift-bot commented Apr 8, 2015

Uh oh!

brenton commented Apr 9, 2015

Uh oh!

brenton commented Apr 9, 2015

Uh oh!

ncdc commented Apr 9, 2015

Uh oh!

brenton commented Apr 9, 2015

Uh oh!

Uh oh!