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 #37400 - Add setting to control validation of host/lifecycle environment/content source coherence #10981

Merged

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Apr 30, 2024

What are the changes introduced in this pull request?

Adds a Setting to allow you to skip running the validation ensure_valid_content_source. In certain edge-case situations, this validation can cause problems with host registrations.

Considerations taken when implementing this change?

Turning this setting off will erase the guardrails introduced in #10524. Therefore, it defaults to on and the vast majority of users should never need to touch it.

Regardless of the value of the setting, the web UI will still look at coherence of the assigned content source/LCE, and you'll still get UI niceties like those pictured in #10524.

What are the testing steps for this pull request?

set up an external smart proxy
Don't sync any environments to it (not even Library)
Register the host to the external smart proxy.

With the setting on, you'll get an error. With the setting off, registration will succeed with no errors.

@jeremylenz jeremylenz force-pushed the 37400-who-needs-content-source-coherence branch from 531bdf4 to d884bbc Compare May 1, 2024 13:36
Copy link
Member

@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.

Works, with the setting on I can register to smart proxies that don't sync LCEs. Without the setting, at registration time, I see:

Validation failed: Content view environment content facets is invalid (HTTP error code 422: Unprocessable Entity)

type: :boolean,
default: true,
full_name: N_('Validate host/lifecycle environment/content source coherence'),
description: N_("Validate that a host's assigned lifecycle environment is synced by the smart proxy from which the host will get its content")
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
description: N_("Validate that a host's assigned lifecycle environment is synced by the smart proxy from which the host will get its content")
description: N_("Validate that a host's assigned lifecycle environment is synced by the smart proxy from which the host will get its content at registration time")

The change content source UI will still block this.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are still some places in the UI that won't, like the legacy Content Host UI and Content host bulk actions. So it's not just registration time.

Copy link
Member

Choose a reason for hiding this comment

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

Think it's worth noting that the change content source UI alone won't respect the setting then?

@jeremylenz
Copy link
Member Author

jeremylenz commented May 1, 2024

Works, with the setting on I can register to smart proxies that don't sync LCEs. Without the setting, at registration time, I see:

Validation failed: Content view environment content facets is invalid (HTTP error code 422: Unprocessable Entity)

Interesting, is that the error that's propagated to subscription-manager? The other day we were actually seeing the validation message itself ("does not sync lifecycle environment xxx") which would be better. It's possible that changed after the multi-CV refactor.

@ianballou
Copy link
Member

@jeremylenz yeah that's how it popped up when I tried registering to a smart proxy that didn't sync the LCE.

@jeremylenz
Copy link
Member Author

Ah yeah, I'm pretty sure it's because that validation moved from content_facet to content_view_environment_content_facet.

@jeremylenz jeremylenz force-pushed the 37400-who-needs-content-source-coherence branch from 845b52f to fbc4402 Compare May 2, 2024 17:03
@jeremylenz jeremylenz force-pushed the 37400-who-needs-content-source-coherence branch from fbc4402 to a839ba7 Compare May 2, 2024 17:13
Copy link
Member

@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.

Works great! I see the right errors in bash after registration and in the logs. Am able to work around the errors with the setting.

@jeremylenz jeremylenz merged commit a1c864a into Katello:master May 2, 2024
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants