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

CP4AIOPS-3113 Store saml remote user configuration separately from sssd lookup #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Aug 23, 2024

Part of ManageIQ/manageiq#23139

saml configuration (mod_auth_mellon) uses a different delimiter from sssd type configurations.

The templates have been changed to reflect this change. The appliance console is now respecting this change

Comment on lines 105 to 100
# legacy systems may still have it stored as the old name
remove_file(HTTPD_CONFIG_DIRECTORY.join("manageiq-remote-user.conf"))
Copy link
Member

Choose a reason for hiding this comment

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

Let's change this comment to be only on the line it comments on. Otherwise it appears like it applies to the whole "paragraph" of code.

Suggested change
# legacy systems may still have it stored as the old name
remove_file(HTTPD_CONFIG_DIRECTORY.join("manageiq-remote-user.conf"))
remove_file(HTTPD_CONFIG_DIRECTORY.join("manageiq-remote-user.conf")) # legacy systems may still have it stored as the old name

Copy link
Member Author

Choose a reason for hiding this comment

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

thnx.
think we're getting away from same line comments (rubocop)
Will change wording to make more clear

Copy link
Member

Choose a reason for hiding this comment

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

I disagree with rubocop on that :)

@@ -1,5 +1,5 @@
module ManageIQ
module ApplianceConsole
VERSION = '9.1.1'.freeze
VERSION = '9.2'.freeze
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
VERSION = '9.2'.freeze
VERSION = '9.2.0'.freeze

@@ -1,5 +1,5 @@
module ManageIQ
module ApplianceConsole
VERSION = '9.1.1'.freeze
VERSION = '10.0'.freeze
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
VERSION = '10.0'.freeze
VERSION = '10.0.0'.freeze

saml configuration (mod_auth_mellon) uses a different delimiter from sssd type configurations.

The templates have been changed to reflect this change.
The appliance console is now respecting this change
@kbrock
Copy link
Member Author

kbrock commented Sep 4, 2024

update:

  • added support for file not present on appliance

update:

  • changed version to 10.0
  • removed comment about version numbers

update:

  • changed version to 10.0.0
  • changed comment to better explain why the 2 deletes are present

@miq-bot
Copy link
Member

miq-bot commented Sep 4, 2024

Checked commit kbrock@f438373 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🍪

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.

3 participants