-
Notifications
You must be signed in to change notification settings - Fork 50
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
enable token based auth #34
base: master
Are you sure you want to change the base?
Conversation
This is the x-lhan's work distilled and applied to current master. C.f. fraunhoferfokus@bc790f8
Hi Alex! Thanks for the PR. The old PR had too many issues to be merged and is more or less deserted by now. However, @jgimenez opened a PR last week (see #33) which addresses exactly this issue. He wanted to open a new, fine grained PR, but I am not aware of its current state. Let me ping him and if he's out, we'll merge this one. |
If this works, I'm happy to use this :) Quickly reviewing this code, it looks cleaner than mine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes that need to be done before this PR can be merged. I am not sure if you want to make all these changes or I should simply cherry pick the changes from #33 and have this PR closed.
"golang.org/x/crypto/ssh/terminal" | ||
|
||
log "github.com/sirupsen/logrus" | ||
"github.com/docker/distribution/context" | ||
schema2 "github.com/docker/distribution/manifest/schema2" | ||
"github.com/docker/distribution/reference" | ||
"github.com/docker/distribution/registry/client" | ||
"github.com/fraunhoferfokus/deckschrubber/util" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you remove this dependency, you also need to remove the file, as it is not used anywhere else.
"golang.org/x/crypto/ssh/terminal" | ||
|
||
log "github.com/sirupsen/logrus" | ||
"github.com/docker/distribution/context" | ||
schema2 "github.com/docker/distribution/manifest/schema2" | ||
"github.com/docker/distribution/reference" | ||
"github.com/docker/distribution/registry/client" | ||
"github.com/fraunhoferfokus/deckschrubber/util" | ||
"github.com/heroku/docker-registry-client/registry" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if *insecure { | ||
transport = &http.Transport{ | ||
TLSClientConfig: &tls.Config{ | ||
InsecureSkipVerify: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should not be hard coded. Again, see how it is solved in #33
Hi yan-foto,
I need to be able to do token based authentication. A while ago x-lhan
made a PR that contained (among other things) necessary changes for
this work. This is the x-lhan's work distilled and applied to current master.
Do you think it would be possible to merge this into your main branch?
C.f. bc790f8
Cheers
Alex