rkt, config: Add loading configuration from user directory #1981

krnowak · 2016-01-15T16:46:46Z

This is rather a complete fix code-wise, though untested. Still need to update the documentation where necessary and probably add some functional tests.

alban · 2016-01-15T17:16:56Z

tests
documentation
changelog

Do we have a test that fetch images from several different users? E.g. 1 image fetched by 1 user, and deleted by another user, etc.

krnowak · 2016-01-18T15:00:16Z

The local configuration directory path is passed down to stage1. Should user config be passed there as well? This would allow us to override networking configuration. Not sure about the overriding semantics, that would be up to the stage1 implementation to decide I guess.

alban · 2016-01-18T15:51:09Z

Non-root users can run rkt fetch but not rkt run so they cannot do anything with stage1. So I don't think stage1 needs to have access to user config.

jonboulle · 2016-01-18T17:57:50Z

So I don't think stage1 needs to have access to user config.

hmm? what about for the aforementioned network example?

krnowak · 2016-01-19T09:27:59Z

Non-root users can run rkt fetch but not rkt run so they cannot do anything with stage1. So I don't think stage1 needs to have access to user config.

I think that the problem here is that you think of --user-config's value as a config provided by non-root user, which is not a point. It is just a name (I'd like to hear suggestions for better names, though, that would convey that it is going to override local config, like local config is overriding system config). You could use --user-config flag as a root like sudo rkt --user-config=${HOME}/test-cfg run coreos.com/etcd:v2.0.10.

jonboulle · 2016-01-19T10:04:48Z

Right. We need this for #1992

krnowak · 2016-01-19T10:50:55Z

ping @robszumski about possible better name for the --user-config flag.

alban · 2016-01-19T10:56:42Z

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Explicitly allow http connections via a new 'http' option to `--insecure-options` ([#1945](https://github.com/coreos/rkt/pull/1945)). Any data and credentials will be sent in the clear.
 - When using `bash`, `rkt` commands can be auto-completed ([#1955](https://github.com/coreos/rkt/pull/1955)).
 - The executables given on the command line via the `--exec` parameters don't need to be absolute paths anymore ([#1953](https://github.com/coreos/rkt/pull/1953)). This change reflects an update in the appc spec since [v0.7.2](https://github.com/appc/spec/releases/tag/v0.7.2). See rkt's [rkt run --exec](https://github.com/coreos/rkt/blob/master/Documentation/subcommands/run.md#overriding-executable-to-launch) documentation.
+- There is a new global flag for specifying the user configuration directory, --user-config. It overrides whatever is configured in system and local configuration directories.


back quotes around --user-config

link to the pull request

link to the doc. "See rkt's Global Options documentation.

why is it useful from a user perspective? E.g. fetching as user while keeping credentials in the user directory. Or using networking plugins installed in a different directory. (Is it correct?)

robszumski · 2016-01-19T19:03:53Z

--user-config sounds ok because it matches the existing flags. Ideally it would indicate that it was a directory, so something like --user-config-dir, but its fine as is.

krnowak · 2016-01-21T10:45:42Z

Do we have a test that fetch images from several different users? E.g. 1 image fetched by 1 user, and deleted by another user, etc.

How about this test - https://github.com/coreos/rkt/blob/master/tests/rkt_non_root_test.go#L91 Is root there somehow special or can be treated as any other users in this case?

alban · 2016-01-21T11:19:24Z

@krnowak ok. So the test already exists :)

krnowak · 2016-01-21T12:00:06Z

Updated, for now I decided not to forward the user config path to stage1. This needs a bit more work.

iaguis · 2016-01-21T12:20:47Z

CHANGELOG.md

@@ -6,6 +6,7 @@
 - When using `bash`, `rkt` commands can be auto-completed ([#1955](https://github.com/coreos/rkt/pull/1955)).
 - The executables given on the command line via the `--exec` parameters don't need to be absolute paths anymore ([#1953](https://github.com/coreos/rkt/pull/1953)). This change reflects an update in the appc spec since [v0.7.2](https://github.com/appc/spec/releases/tag/v0.7.2). See rkt's [rkt run --exec](https://github.com/coreos/rkt/blob/master/Documentation/subcommands/run.md#overriding-executable-to-launch) documentation.
 - Add a `--full` flag to rkt fetch so it returns full hash of the image. ([#1976](https://github.com/coreos/rkt/pull/1976))
+- There is a new global flag for specifying the user configuration directory, `--user-config`. It overrides whatever is configured in system and local configuration directories. Can be useful for specifying different credentials for fetching images without putting them in the globally visible directory like `/etc/rkt`. See rkt's [Global Options](https://github.com/coreos/rkt/blob/master/Documentation/commands.md#global-options) documentation. ([#1981](https://github.com/coreos/rkt/pull/1981))


s/Can be useful/It can be useful/

s/in the globally/in a globally/

iaguis · 2016-01-21T12:23:50Z

LGTM after the nits

krnowak · 2016-01-21T12:28:45Z

About config for stage1 - #2013.

rkt, config: Add loading configuration from user directory

jonboulle added area/configuration needs/review labels Jan 15, 2016

krnowak force-pushed the krnowak/user-config branch from 12d4ae8 to 3b597fa Compare January 18, 2016 15:11

alban added this to the v0.16.0 milestone Jan 18, 2016

alban added the priority/P2 label Jan 18, 2016

alban reviewed Jan 19, 2016
View reviewed changes

alban added needs/some-changes and removed needs/review labels Jan 19, 2016

alban mentioned this pull request Jan 20, 2016

Use the existing configurable local config dir #2005

Merged

krnowak added 3 commits January 21, 2016 12:57

rkt, config: Add loading configuration from user directory

51625bf

docs: Mention new --user-config flag

0286efe

bash: Update completion data with --user-config flag

cf35ce3

krnowak force-pushed the krnowak/user-config branch from e8f2b17 to e5a86c8 Compare January 21, 2016 11:58

iaguis reviewed Jan 21, 2016
View reviewed changes

CHANGELOG.md: Add an entry about --user-config

766743f

krnowak force-pushed the krnowak/user-config branch from e5a86c8 to 766743f Compare January 21, 2016 12:28

iaguis added a commit that referenced this pull request Jan 21, 2016

Merge pull request #1981 from kinvolk/krnowak/user-config

2808525

rkt, config: Add loading configuration from user directory

iaguis merged commit 2808525 into rkt:master Jan 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rkt, config: Add loading configuration from user directory #1981

rkt, config: Add loading configuration from user directory #1981

krnowak commented Jan 15, 2016

alban commented Jan 15, 2016

krnowak commented Jan 18, 2016

alban commented Jan 18, 2016

jonboulle commented Jan 18, 2016

krnowak commented Jan 19, 2016

jonboulle commented Jan 19, 2016

krnowak commented Jan 19, 2016

alban Jan 19, 2016

robszumski commented Jan 19, 2016

krnowak commented Jan 21, 2016

alban commented Jan 21, 2016

krnowak commented Jan 21, 2016

iaguis Jan 21, 2016

iaguis commented Jan 21, 2016

krnowak commented Jan 21, 2016

rkt, config: Add loading configuration from user directory #1981

rkt, config: Add loading configuration from user directory #1981

Conversation

krnowak commented Jan 15, 2016

alban commented Jan 15, 2016

krnowak commented Jan 18, 2016

alban commented Jan 18, 2016

jonboulle commented Jan 18, 2016

krnowak commented Jan 19, 2016

jonboulle commented Jan 19, 2016

krnowak commented Jan 19, 2016

alban Jan 19, 2016

Choose a reason for hiding this comment

robszumski commented Jan 19, 2016

krnowak commented Jan 21, 2016

alban commented Jan 21, 2016

krnowak commented Jan 21, 2016

iaguis Jan 21, 2016

Choose a reason for hiding this comment

iaguis commented Jan 21, 2016

krnowak commented Jan 21, 2016