Skip to content

Add support for mellon auth #135

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Add support for mellon auth #135

wants to merge 15 commits into from

Conversation

bp85
Copy link

@bp85 bp85 commented Jul 11, 2023

  • Add proxy_server configuration option for ood_portal.yml
  • Install apache packages required for mellon
  • creates a script to generate metadata and certs
  • Add mellon configs

@bp85
Copy link
Author

bp85 commented Jul 11, 2023

@treydock I've been using this in prod for well over an year, finally got around to push it upstream.

@treydock
Copy link
Collaborator

@bp85 Generally I don't recommend deploying things that require manual steps, that somewhat defeats the purpose of Puppet. I think one way to achieve full automation is this:

exec { '/usr/local/bin/mellon_ood_metadata.sh':
  creates => [
    "${apache::httpd_dir}/mellon/mellon.cert",
    "${apache::httpd_dir}/mellon/mellon.key",
    "${apache::httpd_dir}/mellon/mellon_metadata.xml",
  ],
  require => File['/usr/local/bin/mellon_ood_metadata.sh'],
  notify   => Class['apache::service'],
}

@treydock
Copy link
Collaborator

I also see some hardcoded paths for Mellon defaults while other places use variables from Apache module. I think all paths should be defined in init.pp , and referenced downstream in various places using maybe the mellon_merged_config variable. This helps ensure if someone changes a variable, for example in this module or Apache module, the things using that path will also reflect that change.

@bp85
Copy link
Author

bp85 commented Mar 27, 2025

@treydock Let me know if this looks good or need any more changes.

@treydock
Copy link
Collaborator

Left a few comments, as I think some changes made to this module in past few months are going to conflict with older changes here.

bp85 and others added 3 commits March 31, 2025 11:18
accept changes

Co-authored-by: treydock <[email protected]>
accept changes

Co-authored-by: treydock <[email protected]>
@bp85 bp85 requested a review from treydock April 2, 2025 15:46
@bp85
Copy link
Author

bp85 commented Apr 21, 2025

Left a few comments, as I think some changes made to this module in past few months are going to conflict with older changes here.

@treydock can you review this PR again? I've made a few more changes accordingly.

@bp85
Copy link
Author

bp85 commented May 12, 2025

@treydock just checking in if you got time to review this!

@bp85
Copy link
Author

bp85 commented Jul 30, 2025

@treydock Can you check and let me know if you can merge this

@treydock treydock added the enhancement New feature or request label Aug 5, 2025
}
'mellon': {
apache::mod { 'auth_mellon':
package => $auth_mellon_package,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable doesn't seem to be defined anywhere.

@treydock
Copy link
Collaborator

treydock commented Aug 5, 2025

I made commits to fix some of the lint issues but the undefined variable for auth_mellon_package I'm not sure how to solve as I don't know the correct package name. If the package name doesn't need to be changed, I'd recommend just don't pass anything to package for the auth_mellon apache::mod defined type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants