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

Let cache.name override cache slug name we compute #1265

Closed
wants to merge 4 commits into from

Conversation

BanzaiMan
Copy link
Contributor

So that the jobs within a build can share caches arbitrarily.

It is not possible to put this change in
Travis::Build::Script#cache_slug, since multiple modules may call
this with super; in such cases the end cache name will be different
from what is given in cache.name.

So that the jobs within a build can share caches arbitrarily.

It is not possible to put this change in
`Travis::Build::Script#cache_slug`, since multiple modules may call
this with `super`; in such cases the end cache name will be different
from what is given in `cache.name`.
@BanzaiMan BanzaiMan deployed to org-staging December 1, 2017 21:33 Active
@BanzaiMan
Copy link
Contributor Author

@BanzaiMan
Copy link
Contributor Author

This resolves travis-ci/travis-ci#7590.

@BanzaiMan BanzaiMan deployed to org-staging January 25, 2018 16:31 Active
@BanzaiMan BanzaiMan temporarily deployed to travis-build-staging January 25, 2018 16:31 Inactive
@BanzaiMan BanzaiMan added the travis-yml PR introduces changes in .travis.yml config keys label Mar 11, 2018
@pat-s
Copy link

pat-s commented Mar 25, 2018

@BanzaiMan any ETA of this feature?

Copy link
Contributor

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

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

For documentation/clarity/maybe-just-for-me, can you list what part of the cache object key will remain consistent even when using cache.name?

@BanzaiMan
Copy link
Contributor Author

@meatballhat

All other cache mechanics remain.

The example configuration used in the test above is:

cache:
  bundler: true
  name: shared

stages:
  - set up cache
  - test
rvm:
  - '2.4'
  - '2.3'
  
jobs:
  include:
   - stage: set up cache

This runs the "Set up cache" stage first (which creates shared.tgz), which will be used by the two jobs in the "Test" stage.

The configurations that need change are those of the form:

cache: bundler

but the change would be consistent with other cache configurations, in that it will be transformed into:

cache:
  bundler: true
  name: shared

The caveat with cache corruption remains. This needs to be documented well.

@meatballhat
Copy link
Contributor

@BanzaiMan Do I understand correctly that each cache will continue to be created under a repository-specific "namespace", regardless of the given cache.name? Do any extra safeguards need to be added to prevent path traversal, e.g.:

cache:
  name: "../../../dastardly-nefarious"

@meatballhat
Copy link
Contributor

@BanzaiMan ooooh, and what about URI-encoding the name?

cache:
  name: "very/special?!cache&name%"

@BanzaiMan
Copy link
Contributor Author

@meatballhat cache_slug (computed, or overridden with name) is passed to a Travis::Build::Script::DirectoryCache::Base object, which computes the S3-compat signature for casher to use. It is not a path to a file on the file system. It is conceivable that we end up uploading a cache to, say, s3://bucket/path/to/../../nefarious.tgz, but that file will be downloaded as fetch.tgz, and expanded.

@BanzaiMan
Copy link
Contributor Author

It is worth testing, for sure.

@BanzaiMan BanzaiMan deployed to org-staging March 27, 2018 17:38 Active
@BanzaiMan BanzaiMan temporarily deployed to travis-build-staging March 27, 2018 17:38 Inactive
@BanzaiMan BanzaiMan deployed to com-staging March 27, 2018 18:48 Active
@BanzaiMan
Copy link
Contributor Author

@meatballhat We normalize the cache names:

args.map! { |arg| arg.to_s.gsub(/[^\w\.\_\-]+/, '') }

@BanzaiMan
Copy link
Contributor Author

@meatballhat
Copy link
Contributor

@BanzaiMan Yep, my concern wasn't with local file path traversal, but potentially poisoning or stealing a cache from a different repository.

@BanzaiMan
Copy link
Contributor Author

It just occurred to me. Another consideration we might add for the future is the read_only attribute, so that the job downloads but does not upload the cache.

@mosyp
Copy link

mosyp commented Apr 18, 2018

Hello, imagine I have first stage with 2 concurrent jobs. Later I have second stages which aggregates caches from first stage. Does this PR will help me when merged or there's another already working solution?

@pat-s
Copy link

pat-s commented Apr 24, 2018

Whats the status here @BanzaiMan? :)

@gobetti
Copy link

gobetti commented May 27, 2018

Hello @BanzaiMan , how can we help to have this merged? Thanks!

gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 29, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 29, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 29, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 29, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 29, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.

Ran `travis setup releases` and signed in with GitHub to generate a new
personal access token and have it encrypted.
Reference: travis-ci/travis-ci#2457
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 30, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.

Ran `travis setup releases` and signed in with GitHub to generate a new
personal access token and have it encrypted.
Reference: travis-ci/travis-ci#2457
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 30, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.

Ran `travis setup releases` and signed in with GitHub to generate a new
personal access token and have it encrypted.
Reference: travis-ci/travis-ci#2457
gobetti added a commit to gobetti/RxCocoaNetworking that referenced this pull request May 30, 2018
There's this issue travis-ci/travis-ci#7590 where Travis intentionally
does not reuse the cache for jobs with different environment variables.
A pull request travis-ci/travis-build#1265 is open for a few months so I
decided to go the other route and avoid using environment variables,
which I discovered to be possible when reading several repositories and
finally finding ReactiveCollections, from which I copied and adapted the
travis and build scripts.

Ran `travis setup releases` and signed in with GitHub to generate a new
personal access token and have it encrypted.
Reference: travis-ci/travis-ci#2457
@BanzaiMan BanzaiMan self-assigned this Oct 17, 2018
@BanzaiMan
Copy link
Contributor Author

We'll have a more comprehensive look at this problem.

@BanzaiMan BanzaiMan closed this Nov 8, 2018
@BanzaiMan BanzaiMan deleted the ha-cache-name-attr branch November 8, 2018 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
travis-yml PR introduces changes in .travis.yml config keys
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants