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

allow pinning shasum of imports #206

Closed
mikemccracken opened this issue Oct 20, 2021 · 5 comments · Fixed by #214
Closed

allow pinning shasum of imports #206

mikemccracken opened this issue Oct 20, 2021 · 5 comments · Fixed by #214
Assignees

Comments

@mikemccracken
Copy link
Contributor

mikemccracken commented Oct 20, 2021

Motivation: building with a given stackerfile should always produce the same results.

import:
  - source: http://$artifactory/place/foo.bin
    dest: /bin/foo
    sha256: 300abcdef...

if sha256 exists, then we check it and fail the build if the source doesn't match it.

we can also add a flag --remote-input-hash-required (bad name) that fails the build if any remote imports do not have the sha256 key. because local imports are likely to be built from the same commit that contains the stacker file, we do not want to force a commit between binary builds and image builds.

we could try to be even smarter about what counts as a local import, where stacker would only count it as local if it's in the same git tree as the stacker file

we could also incorporate #41 along with this since we need to change the syntax anyway

@tych0
Copy link
Collaborator

tych0 commented Oct 21, 2021

related discussion on #105

The feature itself sounds fine to me.

Motivation: building with a given stackerfile should always produce the same results.

I am skeptical that this is really achievable in any real world scenario. People want to install packages, which invokes yum, which sets mtimes, atimes, etc.

I think we could add support to umoci for discarding mtimes, but even then, if you allow invoking things like yum (or any other internet-related stuff that's not done through this import: guaranteed hash mechanism.

@tsnaik
Copy link
Contributor

tsnaik commented Oct 30, 2021

we can also add a flag --remote-input-hash-required (bad name) that fails the build if any remote imports do not have the sha256 key.

Does something like this seem good?

--require-hash                 require each remote import to have a hash provided in stackerfiles

@tych0
Copy link
Collaborator

tych0 commented Oct 30, 2021

If I'm honest I don't know that I love the optionality for this. If people have specified a hash and it's different, then the build should fail. If the hash is expected to change, they should change the hash.

I think that's what your patch in #211 did, so I'm going to go ahead and close this.

@tych0 tych0 closed this as completed Oct 30, 2021
@mikemccracken
Copy link
Contributor Author

@tych0 the extra flag was not to avoid checking hashes, but to tell stacker to fail the build during stackerfile parsing if it finds an import that does not have a hash specified in the file.

we want to enforce a rule that all stackerfiles in a repo must specify hashes.

@tych0
Copy link
Collaborator

tych0 commented Nov 2, 2021

I see, makes sense, thanks for the clarification.

@tych0 tych0 reopened this Nov 2, 2021
tsnaik added a commit to tsnaik/stacker that referenced this issue Nov 3, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

Closes project-stacker#206
tsnaik added a commit to tsnaik/stacker that referenced this issue Nov 3, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

Closes project-stacker#206
tsnaik added a commit to tsnaik/stacker that referenced this issue Nov 3, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

Closes project-stacker#206
tsnaik added a commit to tsnaik/stacker that referenced this issue Nov 3, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

Closes project-stacker#206
tsnaik added a commit to tsnaik/stacker that referenced this issue Nov 4, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

doc: add a line for --require-hash build flag

Closes project-stacker#206
@tych0 tych0 closed this as completed in #214 Nov 4, 2021
tych0 pushed a commit that referenced this issue Nov 4, 2021
when stacker build is called with the flag --require-hash, it will
fail the build if any remote import (http/https) doesn't have hash
in its yaml entry.

--require-hash                  require all remote imports to have a hash provided in stackerfiles

It still allows local or stacker imports without the hash

doc: add a line for --require-hash build flag

Closes #206
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 a pull request may close this issue.

3 participants