Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) #9582

wweellddeerr · 2024-12-17T21:22:17Z

What does this PR change?

It adds the logic for creating a new notification every 3 months for users with disabled SCC data forwarding.
It also includes a new option for enabling SCC data forwarding configuration directly by clicking in the notification link.

GUI diff

DONE

Documentation

No documentation needed, the related warning in docs is already released.
DONE

Test coverage

ℹ️ If a major new functionality is added, it is strongly recommended that tests for the new functionality are added to the Cucumber test suite

No tests: add explanation
No tests: already covered
Unit tests were added
Cucumber tests were added
DONE

Links

Issue(s): https://github.com/SUSE/spacewalk/issues/25778
Port(s): https://github.com/SUSE/spacewalk/pull/26072 https://github.com/SUSE/spacewalk/pull/26073

DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

Re-run test "changelog_test"
Re-run test "backend_unittests_pgsql"
Re-run test "java_pgsql_tests"
Re-run test "schema_migration_test_pgsql"
Re-run test "susemanager_unittests"
Re-run test "javascript_lint"
Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

github-actions · 2024-12-17T21:22:30Z

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9582/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9582/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

github-actions · 2024-12-17T21:22:46Z

Suggested tests to cover this Pull Request

proxy_cobbler_pxeboot
srv_monitoring
srv_rename_hostname
proxy_as_pod_basic_tests
proxy_branch_network
allcli_sanity
srv_notifications
allcli_overview_systems_details
srv_sync_fake_channels
min_cve_id_new_syntax
proxy_container_branch_network
min_deblike_salt_install_package
srv_salt
proxy_container
sle_minion
srv_power_management_redfish
srv_first_settings
buildhost_docker_auth_registry
min_salt_minion_details
min_deblike_ssh
min_rhlike_ssh
min_deblike_openscap_audit
srv_channel_api
srv_disable_local_repos_off
srv_delete_channel_from_ui
min_move_from_and_to_proxy
srv_patches_page
srv_create_repository
srv_user_configuration_salt_states
srv_create_fake_channels
proxy_container_retail_mass_import
proxy_traditional_cobbler_pxeboot
min_salt_formulas_advanced
srv_dist_channel_mapping
srv_reportdb
srv_group_union_intersection
srv_advanced_search
srv_docker_cve_audit
min_rhlike_salt
srv_mainpage
srv_delete_channel_with_tool
minssh_tunnel
srv_enable_sync_products
min_cve_audit
proxy_traditional_branch_network
min_salt_minions_page
min_activationkey
srv_user_api
min_ansible_control_node
srv_salt_download_endpoint
min_salt_formulas
min_bootstrap_ssh_key
srv_user_preferences
srv_disable_scheduled_reposync
srv_power_management_api
srv_cobbler_distro
min_rhlike_salt_install_package_and_patch
srv_scc_user_credentials
srv_virtual_host_manager
allcli_update_activationkeys
min_empty_system_profiles
buildhost_osimage_build_image
min_timezone
allcli_action_chain
srv_restart
min_deblike_salt_install_with_staging
proxy_container_cobbler_pxeboot
srv_cobbler_profile
srv_handle_config_channels_with_ISS_v2
min_rhlike_remote_command
min_recurring_action
srv_logfile
srv_create_activationkey
min_monitoring
minssh_salt_install_package
srv_organization_credentials
srv_task_status_engine
srv_cobbler_buildiso
srv_content_lifecycle
proxy_retail_pxeboot_and_mass_import
min_custom_pkg_download_endpoint
min_action_chain
proxy_traditional
srv_create_fake_repositories
srv_change_task_schedule
srv_sync_products
min_deblike_monitoring
min_change_software_channel
min_retracted_patches
srv_menu_filter
allcli_config_channel
min_config_state_channel_subscriptions
srv_docker_advanced_content_management
min_config_state_channel
srv_add_rocky8_repositories
min_docker_api
proxy_container_retail_pxeboot
min_rhlike_monitoring
srv_maintenance_windows
min_bootstrap_api
srv_manage_channels_page
min_deblike_salt
min_salt_pkgset_beacon
minkvm_guests
allcli_software_channels
srv_payg_ssh_connection
minssh_action_chain
proxy_register_as_minion_with_script
buildhost_docker_build_image
minssh_ansible_control_node
srv_menu
min_rhlike_openscap_audit
srv_datepicker
srv_handle_software_channels_with_ISS_v2
min_check_patches_install
srv_clone_channel_npn
srv_errata_api
srv_channels_add
min_salt_user_states
srv_check_channels_page
srv_docker
min_virthost
min_salt_lock_packages
min_salt_install_package
srv_sync_channels
allcli_system_group
srv_distro_cobbler
min_project_lotus
srv_wait_for_reposync
srv_cobbler_sync
min_bootstrap_negative
min_salt_openscap_audit
sle_ssh_minion
min_salt_migration
srv_change_password
srv_check_sync_source_packages
allcli_reboot
allcli_software_channels_dependencies
min_salt_software_states
srv_users
srv_power_management
buildhost_bootstrap
proxy_traditional_retail_pxeboot
min_ssh_tunnel
min_config_state_channel_api
srv_osimage
srv_check_reposync
min_bootstrap_reactivation
minssh_bootstrap_api
srv_activationkey_api
min_salt_mgrcompat_state
srv_push_package
min_salt_install_with_staging
minssh_move_from_and_to_proxy
min_deblike_remote_command
proxy_traditional_retail_mass_import
srv_custom_system_info
min_bootstrap_script
srv_security
srv_manage_activationkey

rjmateus

LGTM.
I'm not sure if the new property should be add in last, as developed, or as the first to give more visibility.
@admd any comment on this?

CDellaGiusta

LGTM

parlt91

LGTM. Just one question: Do we also plan to show this checkbox for uyuni users?

wweellddeerr · 2025-01-02T19:24:27Z

LGTM. Just one question: Do we also plan to show this checkbox for uyuni users?

Good question, Pascal. I'll leave this for @admd or perhaps @rjmateus to give a final answer. From my POV, while it doesn't seem strictly necessary for Uyuni, Uyuni users can currently change this property, so showing a notification for them shouldn't be an issue if applicable.

admd · 2025-01-06T08:33:22Z

Hi @wweellddeerr thank you for the implementation.

Just a couple of questions without looking into the code, but overall looks good.

If user check/uncheck the checkbox in the UI, would that propagated to that taskomatic job and enable disable it accordingly?
Would it still be possible to disable this notification by adding its type here against java.notifications_type_disabled property, https://github.com/SUSE/spacewalk/blob/6ddaca1962e6170921c51a29170b1007edf31ed2/java/conf/rhn_java.conf#L220 ? If yes, can you please extend the example in that file.

For Uyuni, we should show this warning only if SCC connection is there. I mean if Uyuni instance is connected to SCC, we should show this warning in case user has opted out. And yes, it's fine to show this checkbox to Uyuni users.

rjmateus · 2025-01-07T14:14:39Z

@wweellddeerr please do not merge it without discussing it again. I will arrange a meeting to brainstorm it

rjmateus

LGTM. Thank you for this change Welder

admd

thank you Welder, this looks good.

admd · 2025-01-08T13:41:05Z

java/code/src/com/suse/manager/webui/controllers/admin/EnableSCCDataForwardingController.java

+        else {
+            ConfigureSatelliteCommand csc = CONFIG_ACTION.getCommand(user);
+            csc.updateBoolean(ConfigDefaults.FORWARD_REGISTRATION, Boolean.TRUE);
+            ValidatorError[] verrors = csc.storeConfiguration();


Does the change also reflected in the file(rhn.conf? )?

One more question. We are only changing the property here but if some user actually disabled the taskomatic job, we don't take care of enabling that here, right?

Does the change also reflected in the file(rhn.conf? )?

Yes, it does.

One more question. We are only changing the property here but if some user actually disabled the taskomatic job, we don't take care of enabling that here, right?

If the Taskomatic job is disabled, the user won’t receive the notification because it is created inside the job. We could create a different job specifically for the notification; however, it would still be possible for the user to disable this specific job. So, I don’t think it’s worth it.

About the taskomatic job: I think one thing that we could consider doing here is resetting the schedule to the default one (the one you have after installation), just in case someone might have changed it to only run once per month or so? Not a blocker for this PR though, if it turns out to be complex we can skip it for now.

If I remember correctly we have a different problem. You cannot "disable" a job. In fact it is removed and you cannot get it back. I think we have a separate issue or bug report about it. This problem is bigger than just a changed schedule

https://github.com/SUSE/spacewalk/issues/16996

thank you for the pointer Michael, that one is concerning :(

admd · 2025-01-08T13:41:20Z

java/conf/rhn_java.conf

@@ -214,7 +214,7 @@ java.kiwi_os_image_building_enabled = true
 java.notifications_lifetime = 30

 # Configure the disablement of notification messages by type - example disabling all notification types
-#java.notifications_type_disabled = OnboardingFailed,ChannelSyncFailed,ChannelSyncFinished,CreateBootstrapRepoFailed,StateApplyFailed,UpdateAvailable,SubscriptionWarning
+#java.notifications_type_disabled = OnboardingFailed,ChannelSyncFailed,ChannelSyncFinished,CreateBootstrapRepoFailed,StateApplyFailed,UpdateAvailable,SubscriptionWarning,SCCOptOutWarning


Thinking back again, please remove this from example.

…A-431)

renner

Thank you, this looks really good to me now! 👍

renner · 2025-01-08T14:04:53Z

java/code/src/com/suse/manager/webui/controllers/admin/EnableSCCDataForwardingController.java

+        else {
+            ConfigureSatelliteCommand csc = CONFIG_ACTION.getCommand(user);
+            csc.updateBoolean(ConfigDefaults.FORWARD_REGISTRATION, Boolean.TRUE);
+            ValidatorError[] verrors = csc.storeConfiguration();


About the taskomatic job: I think one thing that we could consider doing here is resetting the schedule to the default one (the one you have after installation), just in case someone might have changed it to only run once per month or so? Not a blocker for this PR though, if it turns out to be complex we can skip it for now.

github-actions bot added java java_lint_checkstyle java_pgsql_tests API ui javascript_lint data-model old-ui labels Dec 17, 2024

github-actions bot added the needs-translation label Dec 17, 2024

wweellddeerr force-pushed the scc-optout-notification-master branch from 44b2b20 to 7a90392 Compare December 17, 2024 21:36

wweellddeerr changed the title ~~Add notification for users with disabled SCC data forwarding~~ Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) Dec 17, 2024

wweellddeerr force-pushed the scc-optout-notification-master branch 2 times, most recently from 2406b37 to 781a516 Compare December 19, 2024 12:10

github-actions bot added the webapp label Dec 19, 2024

wweellddeerr marked this pull request as ready for review December 19, 2024 16:07

wweellddeerr requested review from a team as code owners December 19, 2024 16:07

wweellddeerr requested review from CDellaGiusta and removed request for a team December 19, 2024 16:07

rjmateus previously approved these changes Dec 19, 2024

View reviewed changes

CDellaGiusta previously approved these changes Dec 20, 2024

View reviewed changes

parlt91 previously approved these changes Dec 20, 2024

View reviewed changes

wweellddeerr dismissed stale reviews from parlt91 and CDellaGiusta via 5993d69 January 7, 2025 23:50

wweellddeerr dismissed rjmateus’s stale review via 5993d69 January 7, 2025 23:50

wweellddeerr force-pushed the scc-optout-notification-master branch from 781a516 to 5993d69 Compare January 7, 2025 23:50

github-actions bot added webui-templates spark-router labels Jan 7, 2025

wweellddeerr force-pushed the scc-optout-notification-master branch from 5993d69 to bf0959c Compare January 7, 2025 23:58

wweellddeerr requested a review from rjmateus January 8, 2025 00:06

rjmateus previously approved these changes Jan 8, 2025

View reviewed changes

admd reviewed Jan 8, 2025

View reviewed changes

Add notification for users with disabled SCC data forwarding (jsc#SUM…

9296dff

…A-431)

wweellddeerr dismissed rjmateus’s stale review via 9296dff January 8, 2025 13:57

wweellddeerr force-pushed the scc-optout-notification-master branch from bf0959c to 9296dff Compare January 8, 2025 13:57

renner approved these changes Jan 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) #9582

Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) #9582

wweellddeerr commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

rjmateus left a comment

CDellaGiusta left a comment

parlt91 left a comment

wweellddeerr commented Jan 2, 2025

admd commented Jan 6, 2025 •

edited

Loading

rjmateus commented Jan 7, 2025

rjmateus left a comment

admd left a comment

admd Jan 8, 2025

admd Jan 8, 2025

wweellddeerr Jan 8, 2025

renner Jan 8, 2025

mcalmer Jan 8, 2025

mcalmer Jan 8, 2025

rjmateus Jan 8, 2025

admd Jan 8, 2025

renner left a comment

renner Jan 8, 2025

Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) #9582

Are you sure you want to change the base?

Add notification for users with disabled SCC data forwarding (jsc#SUMA-431) #9582

Conversation

wweellddeerr commented Dec 17, 2024 • edited Loading

What does this PR change?

GUI diff

Documentation

Test coverage

Links

Changelogs

Re-run a test

Before you merge

github-actions bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

rjmateus left a comment

Choose a reason for hiding this comment

CDellaGiusta left a comment

Choose a reason for hiding this comment

parlt91 left a comment

Choose a reason for hiding this comment

wweellddeerr commented Jan 2, 2025

admd commented Jan 6, 2025 • edited Loading

rjmateus commented Jan 7, 2025

rjmateus left a comment

Choose a reason for hiding this comment

admd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wweellddeerr commented Dec 17, 2024 •

edited

Loading

admd commented Jan 6, 2025 •

edited

Loading