Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Oct 12, 2016

@mtrmac @baude PTAL

We'll soon ship a skopeo sub-pkg which will just contain /etc/containers/policy.json and /etc/containers/registries.d/default.yaml so that docker (after projectatomic/docker#200) and atomic can require it in their spec w/o having N specs shipping those files.

Signed-off-by: Antonio Murdaca [email protected]

default.yaml Outdated
# For reading signatures, schema may be http, https, or file.
# For writing signatures, schema may only be file.

# This is the default signature write location for registries.
Copy link
Contributor

Choose a reason for hiding this comment

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

for docker registries, to be pedantic.

Copy link
Member

Choose a reason for hiding this comment

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

container image registries.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, this does not apply to OpenShift.

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Should this have some kind of Makefile integration? As it is, it does nothing and is, at best, an example to be stowed away in docs.

@runcom
Copy link
Member Author

runcom commented Oct 12, 2016

Should this have some kind of Makefile integration? As it is, it does nothing and is, at best, an example to be stowed away in docs.

right, let's install it with make also

@mtrmac
Copy link
Contributor

mtrmac commented Oct 12, 2016

BTW, now thinking about what we do with policy.json, the way we unconditionally overwrite users’ configuration seems pretty risky. Both of these files should be installed only if nothing already exists in the destination.

@runcom
Copy link
Member Author

runcom commented Oct 12, 2016

@mtrmac PTAL, fixed your comments

@runcom
Copy link
Member Author

runcom commented Oct 12, 2016

BTW, now thinking about what we do with policy.json, the way we unconditionally overwrite users’ configuration seems pretty risky. Both of these files should be installed only if nothing already exists in the destination.

we have config(noreplace) in the RPM, I believe who does make from source knows what he's doing

Makefile Outdated
rm -f skopeo docs/*.1

install: install-binary install-docs install-completions
install -d -m 755 ${CONTAINERSSYSCONFIGDIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn’t the -D immediately below make this unnecessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

install -d -m 755 ${CONTAINERSSYSCONFIGDIR}
install -D -m 644 default-policy.json ${CONTAINERSSYSCONFIGDIR}/policy.json
install -d -m 755 ${REGISTRIESDDIR}
install -D -m 644 default.yaml ${REGISTRIESDDIR}/default.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, the line above can now be dropped perhaps.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

# This is the default signature write location for docker registries.
default-docker:
# sigstore: file:///var/lib/atomic/sigstore
sigstore-staging: file:///var/lib/atomic/sigstore
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be creating this directory as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Antonio Murdaca <[email protected]>
Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks! ACK pending tests.

@runcom runcom merged commit 5d589d6 into containers:master Oct 12, 2016
@runcom runcom deleted the add-defyaml branch October 12, 2016 17:19
@runcom
Copy link
Member Author

runcom commented Oct 12, 2016

@mtrmac can you rebase your integrate-all-the-things branch so that it contains this commit? could you ping me once done, thanks!

@mtrmac
Copy link
Contributor

mtrmac commented Oct 12, 2016

@mtrmac can you rebase your integrate-all-the-things branch so that it contains this commit? could you ping me once done, thanks!

Done, sorry for the delay.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants