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

Fix issue where deploy_kwargs is missing. #31073

Merged
merged 3 commits into from
Feb 10, 2016

Conversation

jmoore987
Copy link
Contributor

@cachedout
Copy link
Contributor

@rallytime Can you have a look here please?

@rallytime
Copy link
Contributor

@jmoore987 Is there some data in particular that you're looking for in the return information after the minion is bootstrapped? In other cloud drivers, we return a more curated list than all of deploy_kwargs. We can't return deploy_kwargs directly in this manner here, because that dictionary contains sensitive information.

@rallytime rallytime added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 10, 2016
@jmoore987
Copy link
Contributor Author

@rallytime https://github.com/saltstack/salt/blob/develop/salt/cloud/__init__.py#L2113 just expects master_host or host to be there and raises an exception if they're not, is the problem I am trying to solve.

https://github.com/saltstack/salt/blob/develop/salt/cloud/__init__.py#L2110 will strip out deploy_kwargs if show_deploy_args is set, which appears to be the desired functionality to account for this sensitive information being there.

If this isn't the case, let me know and I can take a look at another driver to see how they handle it, though at quick glance, it appears the AWS driver does it the same way as I am doing here, including things like keys and passwords:
https://github.com/saltstack/salt/blob/develop/salt/cloud/clouds/libcloud_aws.py#L466
through
https://github.com/saltstack/salt/blob/develop/salt/cloud/clouds/libcloud_aws.py#L538

@rallytime
Copy link
Contributor

@jmoore987 Oh, you're right. I was just looking through some of the other drivers and that is how we're handling that there as well, albeit a little differently. If you wouldn't mind cleaning up the very small lint error, then we can get this merged in!

@rallytime rallytime removed the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 10, 2016
Remove some trailing whitespace.
cachedout pushed a commit that referenced this pull request Feb 10, 2016
Fix issue where deploy_kwargs is missing.
@cachedout cachedout merged commit 3699e54 into saltstack:develop Feb 10, 2016
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 this pull request may close these issues.

3 participants