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

_extern_path in fileclient is broken #36371

Closed
nasenbaer13 opened this issue Sep 16, 2016 · 2 comments
Closed

_extern_path in fileclient is broken #36371

nasenbaer13 opened this issue Sep 16, 2016 · 2 comments
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@nasenbaer13
Copy link
Contributor

Description of Issue/Question

_extern_path constructs a path for the file cache but neglects url parts like query strings, fragments and so on.

Setup

Consider a state like:

/target/dir/geoipDB.tar.gz:
  file.managed:
    - makedirs: True
    - source: "https://download.maxmind.com/app/geoip_download?edition_id=some-db&suffix=tar.gz&license_key=secret_key"
    - source_hash: "https://download.maxmind.com/app/geoip_download?edition_id=some-db&suffix=tar.gz.md5&license_key=secret_key"

This state fails because the url in source and source_hash are parsed and map to the same file. For determining the hash, the actual file is copied over the file containing the hash. Determining the hash then fails.

Steps to Reproduce Issue

Create a state with sources only distinct by their query string.

Versions Report

Custom salt based on 2016.3 but the actual bug is still present in https://github.com/saltstack/salt/blob/develop/salt/fileclient.py#L712

Possible solution:

append query strings, fragments etc instead of url_data.path only. I can prepare a patch if this sounds reasonable.

@gtmanfred
Copy link
Contributor

Can you check if this pr fixes your issue?

#36305

Thanks

@gtmanfred gtmanfred added this to the Approved milestone Sep 16, 2016
@gtmanfred gtmanfred added Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Core relates to code central or existential to Salt TEAM Core Confirmed Salt engineer has confirmed bug/feature - often including a MCVE and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Sep 16, 2016
@rallytime
Copy link
Contributor

Since this is fixed via #36305, I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

3 participants