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

Add contents_gpg to file.managed #31006

Open
terminalmage opened this issue Feb 8, 2016 · 13 comments
Open

Add contents_gpg to file.managed #31006

terminalmage opened this issue Feb 8, 2016 · 13 comments
Labels
Feature new functionality including changes to functionality and code refactors, etc. Platform Relates to OS, containers, platform-based utilities like FS, system based apps State-Module
Milestone

Comments

@terminalmage
Copy link
Contributor

The combination of the file_tree ext_pillar and contents_pillar argument to file.managed allow for binary files to be deployed in pillar data. However, to deploy gpg-encrypted contents, a new argument contents_gpg should be added that, if True, pass the contents through the gpg renderer to decrypt them.

Refs: #1543
CC: @fbretel

@terminalmage terminalmage self-assigned this Feb 8, 2016
@terminalmage terminalmage added the Feature new functionality including changes to functionality and code refactors, etc. label Feb 8, 2016
@terminalmage terminalmage added this to the B 3 milestone Feb 8, 2016
@jfindlay jfindlay added State-Module Platform Relates to OS, containers, platform-based utilities like FS, system based apps TEAM Core labels Feb 8, 2016
@fbretel
Copy link
Contributor

fbretel commented Feb 9, 2016

Hi, I very happy to see this new functionality getting attention. Still I feel the approach looks a bit entangled/complex.

I think the original need was to allow sensitive, and possibly binary, data to be stored as files into (and served from) pillar, like ssh keys for instance or x509 keys.
Why as file ? Two reasons: it's a more convenient store than yaml, especially for binary data, and it's easier to re-encrypt when gpg-keys change. It has also been noted elsewhere that sensitive data must go into pillar for not being available to all hosts.

From a user/usability perspective, as evoked in #1543, an intuitive mean would be source: pillar:// or even contents_pillar: file://.
Additionally, it would be nice to keep the consistency with common pillar data, and use renderers equally for yaml data and for files. That is, maybe use template: gpg and better renderer: gpg.

To sum up, I guess I suggest to support files as another built-in source for pillar data, just as yaml data. With this approach, we'd probably win renderer flexibility, environment support, and maybe other functionality that I can't think of. I guess ext_pillar.file_tree would not be needed anymore.
Does that make sense ?

@terminalmage
Copy link
Contributor Author

The idea of using the file_tree external pillar is to allow for ACLs so that a file can not be readable and able to be decrypted by any minion that doesn't need to have that information. Trading convenience for added security, so to speak.

To do it in the way you suggest means that the file is just part of the salt:// namespace and any minion can request the file and, if the GPG key is deployed to it, read its contents.

@fbretel
Copy link
Contributor

fbretel commented Feb 11, 2016

Having followed #1543, I'm well aware that sensitive data should be in pillar :) What in my proposition suggests

that the file is just part of the salt:// namespace
?

I stated:

the original need was to allow sensitive, and possibly binary, data to be stored as files into (and served from) pillar

I think I duplicated #3790.

How about:

# base/pillar/top.sls
prod:
  'host1':
    # for file prod/pillar/ssh/ssh_host_rsa_key.gpg
    - /ssh/ssh_host_rsa_key.gpg

which would be available as pillar ssh:ssh_host_rsa_key.gpg for host1 only ?
Then we need to consider also the renderer at some stage.

@terminalmage
Copy link
Contributor Author

It may be possible, this would be something that would need to be implemented in the common code that compiles both pillar top files and top files for states.

@meggiebot meggiebot modified the milestones: Approved, B 3 Feb 16, 2016
@terminalmage terminalmage added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 23, 2016
@github-abcde
Copy link
Contributor

github-abcde commented Jan 16, 2017

What if we renamed the "template" argument of file.managed to "renderer" and allowed all renderer modules here. That would make the OP's request possible, as well as provide much greater flexibility in what we do with managed files (also considering you could add custom renderer modules).
Edit (clarification): Since all templating engines are renderers anyway.

@terminalmage
Copy link
Contributor Author

terminalmage commented Aug 7, 2017

@github-abcde Renaming the argument would not work, because not all renderers return a string. There are two types of renderers, data renderers and template renderers. Data renderers return a Python dictionary, while template renderers return a string.

What might work is allowing for a pipe syntax similar to what we allow for renderers, so that we can pass the file through multiple template renderers. For example:

/etc/foo.conf:
  file.managed:
    - source: salt://myfiles/foo.conf.gpg
    - template: gpg | jinja

Thoughts?

@github-abcde
Copy link
Contributor

That looks like it will probably work if something similar is done like in salt/template.py. You could just call its compile_template-function with the supplied template-render-pipe as default argument.

@eliasp
Copy link
Contributor

eliasp commented Nov 13, 2018

Related: #18406 (comment)

@stale
Copy link

stale bot commented Jan 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 9, 2020
@github-abcde
Copy link
Contributor

Bump stalebot!

@stale
Copy link

stale bot commented Jan 10, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 10, 2020
@sagetherage
Copy link
Contributor

@terminalmage are you still planning to work on this issue, if not I will unassign.

@terminalmage terminalmage removed their assignment Jan 23, 2020
@terminalmage
Copy link
Contributor Author

Probably not in the near future. I've unassigned myself.

@sagetherage sagetherage removed the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. Platform Relates to OS, containers, platform-based utilities like FS, system based apps State-Module
Projects
None yet
Development

No branches or pull requests

7 participants