implement new docker config file format for authentication by fbuecklers · Pull Request #727 · docker-java/docker-java

fbuecklers · 2016-10-16T21:06:53Z

detect and parse the new config.json used by docker login https://docs.docker.com/engine/reference/commandline/login/
add auth option to the create image command, to lazy pull missing images

Maybe we should use a common implementation of the X-Registry-Auth header logic. Currently it is duplicated in each CommandImpl.

WDYT?

This change is

codecov-io · 2016-11-20T17:39:19Z

Codecov Report

Merging #727 into master will increase coverage by 0.31%.
The diff coverage is 66.43%.

@@            Coverage Diff             @@
##           master     #727      +/-   ##
==========================================
+ Coverage   61.58%   61.89%   +0.31%     
==========================================
  Files         411      410       -1     
  Lines        8172     8152      -20     
  Branches      530      521       -9     
==========================================
+ Hits         5033     5046      +13     
+ Misses       2838     2809      -29     
+ Partials      301      297       -4

Impacted Files	Coverage Δ
.../dockerjava/netty/exec/CreateContainerCmdExec.java	`100% <100%> (ø)`	⬆️
...om/github/dockerjava/core/command/AuthCmdImpl.java	`100% <100%> (ø)`	⬆️
...a/com/github/dockerjava/core/DockerClientImpl.java	`74.25% <100%> (+0.72%)`	⬆️
...github/dockerjava/netty/exec/PullImageCmdExec.java	`100% <100%> (+7.69%)`	⬆️
...thub/dockerjava/core/command/PullImageCmdImpl.java	`84.21% <100%> (+8.02%)`	⬆️
.../com/github/dockerjava/jaxrs/PushImageCmdExec.java	`92.3% <100%> (-0.55%)`	⬇️
.../com/github/dockerjava/jaxrs/PullImageCmdExec.java	`100% <100%> (+7.69%)`	⬆️
...ockerjava/core/command/CreateContainerCmdImpl.java	`53.35% <100%> (+0.63%)`	⬆️
...ithub/dockerjava/jaxrs/CreateContainerCmdExec.java	`100% <100%> (ø)`	⬆️
...github/dockerjava/netty/exec/PushImageCmdExec.java	`92.3% <100%> (-0.55%)`	⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8902e32...b05b1bb. Read the comment docs.

zifik · 2017-04-20T02:05:59Z

Does this help solve bug #316 ?

fbuecklers · 2017-04-20T06:54:46Z

Yep, I use this changes to get the auth information from the new config.json file format.

KostyaSha · 2017-04-23T22:47:29Z

-import com.github.dockerjava.api.model.AuthConfig;
-import com.github.dockerjava.api.model.AuthConfigurations;
-
-public class AuthConfigFile {


seems we can't remove public class in minor release :(

KostyaSha · 2017-04-23T22:56:04Z

+        return withOptionalAuthConfig(authConfig);
+    }
+
+    private CreateContainerCmd withOptionalAuthConfig(AuthConfig authConfig) {


what is the difference?

KostyaSha · 2017-04-23T22:56:09Z

+        return authConfig;
+    }
+
+    public CreateContainerCmd withAuthConfig(AuthConfig authConfig) {


KostyaSha · 2017-04-23T22:57:23Z

Frankly speaking i don't understand what docker did with all this changes, my use case is calls from jenkins/java code with custom credentials, so i'm passing auth for all commands without using login.

I guess this configs are needed if docker-java is used as docker cli analogue?

KostyaSha · 2017-09-06T02:25:03Z

master is open for refactorings, but people added additional hotfixes for auth. could you rebase/review/propose now changes?

fbuecklers · 2017-09-07T21:45:36Z

Hi, yes I will rebase my pull request on weekend. Currently I do not have enough time for it.

KostyaSha · 2017-09-07T22:05:19Z

please wait.. i'm refactoring all tests for master.

fbuecklers · 2017-10-03T11:15:13Z

I have rebased my changes on the latest master, hope that this pull request is fine now

fbuecklers · 2017-10-22T16:16:12Z

Are there any issues on my side, which cause the error?

KostyaSha · 2017-10-24T01:13:44Z

    @CheckForNull
    Boolean isTty();

+    CreateContainerCmd withAuthConfig(AuthConfig authConfig);


why it needed for create container?

Because docker can pull the image while creating a container, if it is missing on the current host. The auth config is then needed if a private registry is used.

add auth option to create image command

… in swarm

fbuecklers · 2017-11-18T16:29:11Z

Ok, I have changed your last suggestions. Anything left?

KostyaSha · 2017-11-18T17:28:12Z

let's try

Such use case existed before. related to docker-java#727

KostyaSha reviewed Apr 23, 2017

View reviewed changes

KostyaSha self-assigned this Apr 24, 2017

KostyaSha removed their assignment Jun 28, 2017

fbuecklers force-pushed the new-auth-config branch from 6732612 to 5f0abac Compare October 3, 2017 11:13

fbuecklers force-pushed the new-auth-config branch from 5f0abac to c60b161 Compare October 4, 2017 09:01

KostyaSha reviewed Oct 24, 2017

View reviewed changes

implement new docker config file format for authentication

4fd7b8a

add auth option to create image command

fbuecklers force-pushed the new-auth-config branch from c60b161 to 4fd7b8a Compare November 4, 2017 00:29

Add more config and auth tests for push / pull and container creation…

cc0b2e0

… in swarm

fbuecklers force-pushed the new-auth-config branch from f9217a9 to cc0b2e0 Compare November 4, 2017 23:13

fbuecklers added 3 commits November 7, 2017 12:55

More Test fixes

fe2c5ee

Adjust the docker api version exception type

9a937a3

Normalize the authConfig handling in the different commands

5139cd8

Small test fixes

b05b1bb

KostyaSha merged commit 97f4827 into docker-java:master Nov 18, 2017

KostyaSha added this to the 3.1.0 milestone Nov 18, 2017

fbuecklers deleted the new-auth-config branch November 21, 2017 11:45

KostyaSha added a commit to KostyaSha/docker-java that referenced this pull request Nov 27, 2018

Fix NPE when file doesn't exist.

80cae55

Such use case existed before. related to docker-java#727

KostyaSha mentioned this pull request Nov 27, 2018

Fix NPE when file doesn't exist. #1138

Merged

Uh oh!

Conversation

fbuecklers commented Oct 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Nov 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zifik commented Apr 20, 2017

Uh oh!

fbuecklers commented Apr 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KostyaSha Apr 23, 2017

Choose a reason for hiding this comment

Uh oh!

KostyaSha Apr 23, 2017

Choose a reason for hiding this comment

Uh oh!

KostyaSha Apr 23, 2017

Choose a reason for hiding this comment

Uh oh!

KostyaSha commented Apr 23, 2017

Uh oh!

KostyaSha commented Sep 6, 2017

Uh oh!

fbuecklers commented Sep 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KostyaSha commented Sep 7, 2017

Uh oh!

fbuecklers commented Oct 3, 2017

Uh oh!

fbuecklers commented Oct 22, 2017

Uh oh!

KostyaSha Oct 24, 2017

Choose a reason for hiding this comment

Uh oh!

fbuecklers Oct 24, 2017

Choose a reason for hiding this comment

Uh oh!

fbuecklers commented Nov 18, 2017

Uh oh!

KostyaSha commented Nov 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fbuecklers commented Oct 16, 2016 •

edited

Loading

codecov-io commented Nov 20, 2016 •

edited

Loading

fbuecklers commented Apr 20, 2017 •

edited

Loading

fbuecklers commented Sep 7, 2017 •

edited

Loading