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

Allow user modification of access and error log in vhost. #108

Closed
wants to merge 5 commits into from
Closed

Allow user modification of access and error log in vhost. #108

wants to merge 5 commits into from

Conversation

vrillusions
Copy link
Contributor

Motivation for this is some vhosts I didn't want to log anything. The way I did it feels clunky to me but I try to favor cleaner templates over manifests. Was going to add similar to logic to server logs for completeness. First wanted to check that this is alright vs doing something where logic is part template and part manifest (ie keep the assembled default log in template and use if blocks around it.

@vrillusions
Copy link
Contributor Author

Also worth mentioning there's no need to do this to to location resource as the additional config thing can add them to turn off logging per location

@abraham1901
Copy link
Contributor

Hello, @vrillusions
It's really good changes. Thanks.
Please add new variable to vhost template with ssl: vhost_ssl_header.erb

@vrillusions
Copy link
Contributor Author

OK made change to ssl template as well as making sure it adds the same "ssl-" prefix to logname.

@vrillusions
Copy link
Contributor Author

Actually hold off on pulling, both access_log and error_log can take optional parameters after it so the validate path check shouldn't be in there.

(edit)OK that should be the last commit.

@vrillusions
Copy link
Contributor Author

OK just to get this all good and done with I did the same for logs at server level. Although right now to make it clear where the logs are located in the config they're not consistent variable names. the error log is called "nginx_error_log" and then the access log is called "http_access_log" as that's the section its in.

@abraham1901
Copy link
Contributor

Thanks for you code. I'm testing, fix simple bug and add new PR #120

Conflicts:
	manifests/config.pp
	manifests/params.pp
@vrillusions
Copy link
Contributor Author

fwiw just did a fresh merge of master and fixed a couple minor conflicts but otherwise is working as expected.

@vrillusions
Copy link
Contributor Author

I verified my fix is in latest master now that #120 is merged.

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.

2 participants