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

Fixes #37204 - Move proxy params to correct location in virt-who.conf #189

Merged
merged 1 commit into from Feb 27, 2024

Conversation

chris1984
Copy link
Member

@chris1984 chris1984 commented Feb 22, 2024

What are the changes introduced in this pull request?

  • Fixed an issue where the HTTP PROXY dropdown was showing the full URL instead of name like other places in Foreman causing the UI to look odd
  • Moved the no_proxy under [system_environment]
  • Looking at the man page you only need to specify no_proxy if you want to ignore proxies and it should not be there by default so fixed the logic that was causing it to be there even if the field was empty and updated the tests
  • moved http/s_proxy under [system_environment]

What are the testing steps for this pull request?

  • Check out PR
  • Create an http and https proxy, does not have to be valid just use a fake url
  • Create and deploy a config without any proxy info and make sure no_proxy is not in the /etc/virt-who.conf config
  • Edit that config and add * into the ignore_proxy UI field, save and deploy the config check and see if you see no_proxy=* in the /etc/virt-who.conf config
  • Edit that config and chose an HTTP Proxy from the dropdown, save and deploy the config and make sure you see http_proxy under [system_environment]
  • Edit that config and chose an HTTPS Proxy from the dropdown, save and deploy the config and make sure you see https_proxy under [system_environment]

If you don't want to deploy the config, you can just look at the script and see what it would put in /etc/virt-who.conf that is what I did when testing the changes before I tried to deploy them.

With my PR it should look like this for each scenario:

HTTPS Proxy:

### This configuration file is managed via the virt-who configure plugin
### manual edits will be deleted.
[global]
debug=1
interval=86400
oneshot=False

[system_environment]
https_proxy=https://admin:changeme@proxy.satellite.lab.eng.rdu2.redhat.com:3128

HTTP Proxy:

### This configuration file is managed via the virt-who configure plugin
### manual edits will be deleted.
[global]
debug=1
interval=86400
oneshot=False

[system_environment]
http_proxy=http://admin:changeme@proxy.satellite.lab.eng.rdu2.redhat.com:3128

Ignore Proxy:

### This configuration file is managed via the virt-who configure plugin
### manual edits will be deleted.
[global]
debug=1
interval=86400
oneshot=False

[system_environment]
no_proxy=*

No Proxy:

### This configuration file is managed via the virt-who configure plugin
### manual edits will be deleted.
[global]
debug=1
interval=86400
oneshot=False

[system_environment]

Copy link
Contributor

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

I tested each combination of proxies and ignore proxy settings, everything seems to be sane. Just had one thing I wanted to make sure was intended:

app/models/foreman_virt_who_configure/output_generator.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Working well for me now!

Verified that no_proxy doesn't show up unless the user specifies it and that there is only one.

@chris1984 chris1984 merged commit 16869e4 into theforeman:master Feb 27, 2024
7 of 10 checks passed
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.

None yet

2 participants