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

document log levels and warn on all logging below info #35356

Merged
merged 5 commits into from
Aug 11, 2016

Conversation

jfindlay
Copy link
Contributor

@jfindlay jfindlay commented Aug 10, 2016

What does this PR do?

Document log levels and warn about sensitive data on all logging below info

I also wanted to warn on insecure levels of log_level_logfile, but this is not possible at present because of the following complications:

Running a check in salt.utils.verify.verify_log against log_level_logfile results in that config being set to None because that is what it is in salt.config.

salt.minion and salt.utils.cloud both set log_level_logfile to info as advertised by the logging docs and the minion config docs, but these are all incorrect and preempted by salt.utils.parsers, which sets log_level_logfile indiscriminately to warning. I have also verified this with a 2016.3.2 install.

If advised, I will submit another pull request against develop unifying these contradictory defaults to warning or info and update the release notes. I think warning would be better since that is what effectively happens and we should change salt.minion and salt.utils.cloud and the docs to that.

In my opinion, salt.config should be considered the authority on option defaults so that, for example, salt.utils.parsers can source its defaults from there rather than provide duplicate or contradictory defaults. What do you think, @s0undt3ch?

What issues does this PR fix or reference?

none

Tests written?

Yes

@jfindlay jfindlay force-pushed the log_levels branch 2 times, most recently from b84eabf to 3b57b61 Compare August 10, 2016 20:01
@cachedout cachedout merged commit 0fcfc70 into saltstack:2016.3 Aug 11, 2016
@s0undt3ch
Copy link
Collaborator

Agreed, let's unify.

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.

None yet

3 participants