OCIR Authentication integration #37
Conversation
MadalinaPatrichi
commented
Apr 1, 2019
MadalinaPatrichi
commented
- Modified packaging mechanism to use dep instead of Godeps (Godeps is deprecated)
- Added config to authenticate against OCIR
There was a problem hiding this comment.
Major changes I'd suggest here :
- Skip 'dep' and go straight to 'go mod'
- Don't make this env vars specific to oracle, let them be set for all repos and set the username/password (do /should these also work on pulling images?)
- Don't make the OCIR code specific to IAD - make it region general
I'm not sure i follow why you need to have the pre-canned redirect - is this just saving an extract HTTP call?
| @@ -54,6 +56,15 @@ func parseRepoInfo(remote string, docker bool) (*RepoInfo, error) { | |||
| r.Auth = "https://auth.docker.io/token" | |||
| r.Service = "registry.docker.io" | |||
| r.Docker = true | |||
| } else if r.Host == "iad.ocir.io" { | |||
There was a problem hiding this comment.
This should be generalised to all regions
| @@ -0,0 +1,86 @@ | |||
| # Gopkg.toml example | |||
There was a problem hiding this comment.
you can remove dep now - just run go mod init and delete the Gopkg.toml and lock
When you do that you need to make sure that blackfriday ( is referenced with a '/v2' suffix :
github.com/russross/blackfriday/v2 v2.0.1
not
github.com/russross/blackfriday v2.0.1
| @@ -20,4 +20,6 @@ VOLUME /write | |||
|
|
|||
| WORKDIR /write | |||
|
|
|||
| ENV IAD_USERNAME="tenant/username" | |||
There was a problem hiding this comment.
Is pre-declaring these in the image the right thing?
wont this mean it will always default to sending an incorrect username and password when it tries to push from docker.
maybe the example docker script in the README.md should just pass these on from the caller in the docker run command (docker run -e DOCKER_USERNAME -e DOCKER_PASSWORD ) ?
| "path" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/Sirupsen/logrus" | ||
| gdigest "github.com/opencontainers/go-digest" | ||
| common "github.com/oracle/oci-go-sdk/common" |
There was a problem hiding this comment.
the prefix is unnecessary here
| @@ -90,6 +101,50 @@ func NewRegistryClient(insecure bool) *RegistryClient { | |||
| return &RegistryClient{http.Client{Transport: tr}} | |||
| } | |||
|
|
|||
| func (r *RegistryClient) SignRequest(info *RepoInfo, request *http.Request) { | |||
There was a problem hiding this comment.
Is this ever used? If not remove and remove go SDK dep
It would be neat to get the token via OCI auth where supported (generally remove the need for Auth tokens and use standard signing creds) but I don't see this being used?
| func getResponseMessage(response *http.Response) string { | ||
| var buf bytes.Buffer | ||
| if _, err := buf.ReadFrom(response.Body); err != nil { | ||
| logrus.Warnf("Failed to read response body: %v", err) |
There was a problem hiding this comment.
Definitely shouldn't be swallowing an error here - make it bubble or make the logrus fatal.
| r.Docker = true | ||
| if data.User == nil { | ||
|
|
||
| r.Username = os.Getenv("IAD_USERNAME") |
There was a problem hiding this comment.
If you want to use env vars to pass these on then I think they should just be "DOCKER_USERNAME" "DOCKER_PASSWORD" and you should set them further down
having the ability to not require passwords in the URL would be useful , especially for pulling but I don't think it's Oracle specific.
