Skip to content
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

Respect XDG_CONFIG_HOME for policy.json and cni #248

Merged
merged 2 commits into from
Aug 10, 2020

Conversation

davidscherer
Copy link

Use the XDG_CONFIG_HOME environment variable, if it is defined, to locate policy.json and the cni plugins directory in rootless mode. This is the standard way to locate user-specific configuration files.

XDG_CONFIG_HOME is already respected in rootless mode for configuration files such as containers.conf, storage.conf, and registry.conf but here ~/.config was hardcoded.

Use XDG_CONFIG_HOME if it is defined.

XDG_CONFIG_HOME is already respected for configuration files such as containers.conf, storage.conf, and registry.conf but here `~/.config` was hardcoded.

Signed-off-by: David Scherer <david.scherer@antithesis.com>
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: davidscherer
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mheon
Copy link
Member

mheon commented Aug 8, 2020

Signature policy path changes LGTM, but CNI is unusable without root, so there's no sense in making it vary per-user.

@davidscherer
Copy link
Author

@mheon it seems like your disagreement is with (previously merged) #221 which claims that it is "preparing" common for rootless CNI.

@davidscherer
Copy link
Author

Unfortunately, I have discovered that while this patch appears to make podman behave as I expect, buildah is apparently relying instead on the very similar logic in https://github.com/containers/image/blob/3e7b89e181cc724370de44856fe06092c20cbd15/signature/policy_config.go#L60

@mheon
Copy link
Member

mheon commented Aug 8, 2020

Ah, exciting - I thought that was a lot further off.

@rhatdan
Copy link
Member

rhatdan commented Aug 9, 2020

Please use "github.com/containers/storage/pkg/homedir" config.GetConfigHome()
rather then reimplementing it.

Signed-off-by: David Scherer <david.scherer@antithesis.com>
@davidscherer
Copy link
Author

It has nothing to do with this PR, but probably rootlessConfigPath() next door in config.go should also use homedir.GetConfigHome()

@rhatdan
Copy link
Member

rhatdan commented Aug 10, 2020

@davidscherer Yes could you open a PR to fix this.

@rhatdan
Copy link
Member

rhatdan commented Aug 10, 2020

LGTM

@rhatdan rhatdan merged commit ecb9aa6 into containers:master Aug 10, 2020
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.

None yet

4 participants