read from /etc/container/rhel/secrets also #187
Conversation
Signed-off-by: Antonio Murdaca <[email protected]>
|
Do we want to create this directory on install, or just document that the user can create the directory to override the default? Or should we just read both directories? |
I'd go with shipping the directory as we do with the other so people are aware of this functionality on install but this is just my 2c. @cgwalters wdyt? |
|
If we ship the directory now and the directory is empty then it will break the default secret passing. Since it only grabs /etc/containers/rhel/secrets if it exists. |
|
🚲 Can we drop the |
| @@ -81,5 +86,8 @@ func readFile(root, name string) ([]SecretData, error) { | |||
| } | |||
|
|
|||
| func getHostSecretData() ([]SecretData, error) { | |||
| return readAll("/usr/share/rhel/secrets", "") | |||
| if _, err := os.Stat(overrideDir); err == nil { | |||
There was a problem hiding this comment.
Hmm...I was thinking more like individual files with the same name override (systemd like), whereas this seems to be "if /etc exists, it wins entirely", so users would have to copy over data from /usr that they want to keep.
There was a problem hiding this comment.
So we really need a union of the files in both directories with /etc overriding content in /usr/share.
There was a problem hiding this comment.
@cgwalters I guess we want this union just for rhsm and rhel7.repo - does /etc/pki/entitlement need this?
There was a problem hiding this comment.
@rhatdan @cgwalters case:
/usr/share/rhel/secrets/rhsm/ <- not empty
/etc/container/rhsm/ <- EMPTY
what's the end result in the container?
an empty dir or the merge? I'd say the empty dir, the other way is too much maybe or not?
There was a problem hiding this comment.
Well if we went with the systemd way and had
/usr/share/rhel/secretes/rhsm/
filea, fileb, filec, filed
/etc/container/rhsm
filec,filef
We would end up with
filea, fileb, filed for /usr/share and filec, filef from /etc/container/
There was a problem hiding this comment.
alright so it's a complete merge overriding from /etc/container/ and in case of an empty dir in /etc/container/ we grab everything from /usr/share/rhel/secrets/
no strong opinion on this, @rhatdan ? |
|
Yes drop it. |
Fix #186
@rhatdan @cgwalters @parthaa PTAL
docker.specto include/etc/container/rhel/secretsin%filesas we do for/usr/share/rhel/secrets