Skip to content

Migrate .dockercfg to .docker/config.json and support for HTTP Headers#12009

Merged
LK4D4 merged 1 commit intomoby:masterfrom
duglin:AddConfig
Apr 20, 2015
Merged

Migrate .dockercfg to .docker/config.json and support for HTTP Headers#12009
LK4D4 merged 1 commit intomoby:masterfrom
duglin:AddConfig

Conversation

@duglin
Copy link
Copy Markdown
Contributor

@duglin duglin commented Apr 1, 2015

This PR does the following:

  • migrated ~/.dockerfg to ~/.docker/config.json. The data is migrated
    but the old file remains in case its needed
  • moves the auth json in that fie into an "auth" property so we can add new
    top-level properties w/o messing with the auth stuff
  • adds support for an HttpHeaders property in ~/.docker/config.json
    which adds these http headers to all msgs from the cli

In a follow-on PR I'll move the config file process out from under
"registry" since it not specific to that any more. I didn't do it here
because I wanted the diff to be smaller so people can make sure I didn't
break/miss any auth code during my edits.

Signed-off-by: Doug Davis dug@us.ibm.com

@GordonTheTurtle
Copy link
Copy Markdown

These files are not properly gofmt'd:
api/client/utils.go
Please reformat the above files using gofmt -s -w and amend to the commit the result.

@duglin duglin force-pushed the AddConfig branch 3 times, most recently from ba6b1be to 0ea4505 Compare April 2, 2015 00:18
@duglin
Copy link
Copy Markdown
Contributor Author

duglin commented Apr 2, 2015

ping @jfrazelle @tiborvass @crosbymichael

@tiborvass
Copy link
Copy Markdown
Contributor

@duglin it's interesting but I'm not sure I understand the usecase. What's the problem this is solving?

@duglin
Copy link
Copy Markdown
Contributor Author

duglin commented Apr 2, 2015

@tiborvass there are two things at play here:
1 - the more generic need for a config file that 3rd party tools can modify to change the behavior of the CLI. I think a 'DockerHost' property is the best example of this. Some tooling (perhaps even Machine) needs to point the user's CLI to a particular docker host. Modifying their env vars for them or asking them to do it, or use -H, isn't ideal. So, a config file can be modified instead. I would like to add "DockerHost" support but based on previous chats, for some reason, that might be controversial so I left if out for now.

2 - in our env we have a (sort of) proxy sitting in-front of a set of docker hosts. This proxy will do certain things including auth processing. We want our users to continue to use the default docker CLI to interact with our backends but in order for the auth stuff to work we need some additional auth http headers in all of the CLI messages. So, we'll have some a tool (like a "login" exe) that the user to run to generate the proper auth token and then that tool will modify the config file to specify what http headers the docker cli should then include. So, once the user is logged-in they can then use the Docker cli as they normally would.

@tianon
Copy link
Copy Markdown
Member

tianon commented Apr 3, 2015

Sorry for the premature code review move -- this still needs design review.

I'm +1 on the idea of ~/.docker/config.json, but I personally think that we ought to do cascading config from /etc/docker/config.json to ~/.docker/config.json to some command-line flags (like SSH does, for example), so I only see this as part of a bigger whole. Do you see those kinds of things as simple additions later?

How do we prevent the community at large from building tools to parse and modify this file while we're at it, since there won't be any standard for ensuring that two tools don't try to write it at the same time before we get around to doing so?

@duglin
Copy link
Copy Markdown
Contributor Author

duglin commented Apr 3, 2015

I'm ok with both concepts (adding /etc/docker/config.json into the cascade && some kind of API/mechanism/something to help edit these files). But I think both can be added later.
Right now the CLI doesn't look at /etc/docker/... at all so that would a new concept and easy to add to the mix. We don't even have any overlapping config data between ~/.docker/config.json and anything else yet :-)
New tooling/something to help edit the files can be added in a subsequent PR too.

@duglin
Copy link
Copy Markdown
Contributor Author

duglin commented Apr 3, 2015

@crosbymichael since this is a UX change, can you chime in here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Configuration Files

The Docker command line stores its configuration files in a directory called
.docker within your HOME directory. Docker manages most of the files in
.docker and you should not modify them. However, you can modify the
.docker/config.json file to control certain aspects of how the docker
command behaves.

Currently, you can modify the docker command behavior using environment variables or command-line options. You can also use options within config.json to modify that same behavior. When using these mechanisms, you must keep in mind the order of precedence among them. Command line options override environment variables and environment variables override properties you specify in a config.json file.

The config.json file stores a JSON encoding of a single HttpHeaders
property. The property specifies a set of headers to include in all
messages sent from the Docker client to the daemon. Docker does not try to
interpret or understand these header; it simply puts them into the messages.
Docker does not allow these headers to change any headings it sets for itself.

Following is a sample config.json file:

{
  "HttpHeaders: {
    "MyHeader": "MyValue"
  }
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@moxiegirl ok - picked up your edits with just a minor tweak to the start of the 2nd paragraph. See if you're ok with that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh I also changed "headings" to "headers" in the last para.

@moxiegirl
Copy link
Copy Markdown
Contributor

LGTM

ping @docker/docs-owners

@fredlf
Copy link
Copy Markdown
Contributor

fredlf commented Apr 20, 2015

Docs LGTM with @moxiegirl 's revision.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@duglin Looks like you created an incomplete email address after changing it from @gmail.com

Not sure it's a problem here, just noting it, just in case :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure how I managed to do that but I fixed it. thanks!

On Apr 20, 2015, at 3:56 PM, Sebastiaan van Stijn notifications@github.com wrote:

In registry/config_file_test.go #12009 (comment):

+func TestOldJson(t *testing.T) {

  • if runtime.GOOS == "windows" {
  •   return
    
  • }
  • tmpHome, _ := ioutil.TempDir("", "config-test")
  • defer os.RemoveAll(tmpHome)
  • homeKey := homedir.Key()
  • homeVal := homedir.Get()
  • defer func() { os.Setenv(homeKey, homeVal) }()
  • os.Setenv(homeKey, tmpHome)
  • fn := filepath.Join(tmpHome, OLD_CONFIGFILE)
  • js := {"https://index.docker.io/v1/":{"auth":"am9lam9lOmhlbGxv","email":"@example.com"}}
    @duglin https://github.com/duglin Looks like you created an incomplete email address after changing it from @gmail.com

Not sure it's a problem here, just noting it, just in case :)


Reply to this email directly or view it on GitHub https://github.com/docker/docker/pull/12009/files#r28722782.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy-pasta-slippo 😄 👍

This PR does the following:
- migrated ~/.dockerfg to ~/.docker/config.json. The data is migrated
  but the old file remains in case its needed
- moves the auth json in that fie into an "auth" property so we can add new
  top-level properties w/o messing with the auth stuff
- adds support for an HttpHeaders property in ~/.docker/config.json
  which adds these http headers to all msgs from the cli

In a follow-on PR I'll move the config file process out from under
"registry" since it not specific to that any more. I didn't do it here
because I wanted the diff to be smaller so people can make sure I didn't
break/miss any auth code during my edits.

Signed-off-by: Doug Davis <dug@us.ibm.com>
@moxiegirl
Copy link
Copy Markdown
Contributor

LGTM

LK4D4 added a commit that referenced this pull request Apr 20, 2015
Migrate .dockercfg to .docker/config.json and support for HTTP Headers
@LK4D4 LK4D4 merged commit 08ef006 into moby:master Apr 20, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Argh was still testing this fix when the merge came in. This permission causes problems and needs to be 0700.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok - I can easily change that. But I'm curious, why do you need +x ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Without the x bit traversing will still throw a permission, writing to "~/.docker/config.json" failed and even attempting to touch is not allowed.

$ ls -ld ~/.docker
drw-------. 2 derek derek 4096 Apr 20 12:18 /home/derek/.docker
$ touch ~/.docker/config.json
touch: cannot touch ‘/home/derek/.docker/config.json’: Permission denied

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's what I would have expected. However, if you look at this branch: https://github.com/duglin/docker/tree/TweakConfigPerms
I added a testcase to have Save() create the dir and when I left the code at 0600 it still worked - even was able to read the new config file. Is the next test wrong?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

could it be because the test is running as root? dunno if it is or not, just speculating.

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.