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

Arbitrary apache conf #3293

Merged
merged 5 commits into from
Jan 17, 2024
Merged

Arbitrary apache conf #3293

merged 5 commits into from
Jan 17, 2024

Conversation

johrstrom
Copy link
Contributor

This adds the ability to add arbitrary things to the location and vhost in our apache config.

Fixes #708
Fixes #3240

Though now re-reading 708 I'm seeing multiline strings may not be the best option - I just want to be sure folks can add XML structures as well as single line directives.

@CSC-swesters
Copy link
Contributor

Thank you for working on this issue!

I agree with Treydock's comment: #708 (comment)
The support for a list of lines in the YAML would be valuable to us, since we use Puppet for some of our environments.

I suppose you could support both ways by checking for the data type in the Ruby template? If the type of the @custom_vhost_directives variable is a string, do what your PR does currently. And if it's a list, just do @custom_vhost_directives.each do |line| instead, I suppose.

This would require some additional test cases, but I think both ways could be supported.

@johrstrom
Copy link
Contributor Author

Thanks, yea I changed to arrays of strings. I think it'll be OK to just support one thing - I'm testing here for correct indentation and I think that's the only thing that would be required here - that things, say in a new are formatted correctly.

Copy link
Contributor

@Oglopf Oglopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good, but there is a merge conflict in the spec file for a test. Should be fine after that gets fixed.

@johrstrom johrstrom force-pushed the arbitrary-apache-conf branch from 16adcad to cad8340 Compare January 17, 2024 18:47
Copy link
Contributor

@Oglopf Oglopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@johrstrom johrstrom merged commit a61aef6 into master Jan 17, 2024
20 checks passed
@johrstrom johrstrom deleted the arbitrary-apache-conf branch January 17, 2024 21:48
@xpillons
Copy link

xpillons commented Apr 2, 2024

@johrstrom in which release this is now included ? I don't find any references in the changelogs.

@johrstrom
Copy link
Contributor Author

It's in 3.1

https://osc.github.io/ood-documentation/latest/release-notes/v3.1-release-notes.html#updated-apache-options

And just to be sure I checked the changelog too.

- Sites can now set arbitrary vhost and location directives in Arbitrary apache conf [3293](https://github.com/OSC/ondemand/pull/3293).

@xpillons
Copy link

xpillons commented Apr 2, 2024

Thank you for confirming, I was looking at the release change log https://github.com/OSC/ondemand/releases not the one in the documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants