[FEATURE REQUEST] Better caching filename control #58888

Open ghost opened 3 years ago

Is your feature request related to a problem? Please describe. I want to download GitLab artifacts. GitLab allows the token present in the header or query parameter. I can't use the query parameter because it will be copied to the cached filename on the minion.

Describe the solution you'd like Add 'headers' (dictionary) parameter to state.

Describe alternatives you've considered No alternatives I think.

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at core@saltstack.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Hi @ViktorPegy thanks for the request! A bit of follow up - Why is the query parameter in the filename undesirable? Due to leaking secrets? Or because the token might change? If you could flesh this out a bit more and describe in more detail (obviously with tokens redacted and sample path names) what you're currently doing, what you want to have working, and what Salt is doing instead, that would be fantastic! Thanks again!

Hi @waynew! Thanks for the reply. Query parameter will be copied into the minion cache filename as is, this is dangerous due to leaking secret token. And if we change the secret token, Salt will redownload the file, even it's not changed.

Sample state configuration:

archive.extracted:
  - name: /tmp/archive_dir
  - source: https://example.com/path/to/archive.gz?secret_token=secret_token

Then, the cache filename will be <cache_dir>/example.com/path/to/archive.gz?secret_token=secret_token (URL-encoded)

What I would like:

archive.extracted:
  - name: /tmp/archive_dir
  - source: https://example.com/path/to/archive.gz
  - headers:
      X-Secret-Token: secret_token

The current workaround is to use proxy with basic authorization.

Looked into this a bit closer, and it appears that file.cached, and maybe localclient(?) doesn't really offer a way to skip adding the querystring. In fact, even adding headers would be a pretty wild process to add this capability.

It looks like you could workaround this by defining your own cache and using module.run something like this:

cache archive:
  module.run:
    - name: cp.get_url
    - path: "http://localhost:8000/fun.tar"
    - dest: /tmp/mycache/fun.tar
    - makedirs: True
    - source_hash: sha256=208df0b70526f9d56e3f8b627f3106d09d7d03cebbc44cf8eb6b6ddc7ccc83d9

That would require some other shenanigans to get it to really work the way I certainly expect it to run.

There may be a better way to make this functionality more generally useful. TBH what I would love to see is the ability to override the behavior, perhaps in salt/fileclient.py - currently it has this block:

if url_data.query:
    file_name = "-".join([url_data.path, url_data.query])
else:
    file_name = url_data.path

I'd love to see something like curl's -O flag, where the filename comes from the remote protocol :thinking: (or at the very least, a way to tell it to not include the querystring in the filename)

saltstack / salt

[FEATURE REQUEST] Better caching filename control #58888