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

Define SCL package name for mod_ldap #1893

Merged
merged 1 commit into from
Apr 23, 2019
Merged

Conversation

treydock
Copy link
Contributor

No description provided.

@turnopil
Copy link
Contributor

turnopil commented Apr 12, 2019

Maybe would be a good decision add support for Amazon AMIs because in AWS apache2.4 mod_ldap package called 'mod24_ldap' but for 2.2 - mod_authz_ldap

The error
ESC[1;31mError: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, 'regsubst' parameter 'target' expects a value of type Array or String, got Undef (file: /etc/puppetlabs/code/modules/apache/manifests/mod.pp, line: 55, column: 19) (file: /etc/puppetlabs/code/modules/apache/manifests/mod/ldap.pp, line: 16) on node *******[0m

@lionce
Copy link
Contributor

lionce commented Apr 19, 2019

Hello @treydock ,

Thank you for your contribution. In order to merge this PR, please resolve the conflicts.

Cheers!

@treydock
Copy link
Contributor Author

@lionce Looks like the acceptance tests for SCL logic were removed but there are no corresponding unit tests. 6006ca2 leads me to believe this was intentional but I'm not seeing unit tests to replace the acceptance tests. Should I go ahead and remove my changes that conflict even though no test coverage?

@lionce
Copy link
Contributor

lionce commented Apr 22, 2019

Hello @treydock ,

Yes, we removed some acceptance tests because they were testing the connection to a specific port, or the server answer. Most of them were replaced by unit tests. In order to increase the test coverage, I would really appreciate if you will add a unit test to cover this case. Many thanks!

Cheers!

@treydock
Copy link
Contributor Author

@lionce Added unit tests for changes in this PR.

@treydock
Copy link
Contributor Author

Also had to add $conf_enabled as needed by main apache class and was throwing undefined variable errors.

@lionce
Copy link
Contributor

lionce commented Apr 23, 2019

Hello @treydock ,

Thank you for this PR. It looks good to me!

Cheers

@lionce lionce merged commit 0df40f2 into puppetlabs:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants