Skip to content

Conversation

@bshephar
Copy link

Enhancement:
All values for rhc_organization should be interger values. This change asserts that requirement by ensuring the value is an integer before trying to register.

Reason:
In situations where we're unmarshaling data from json to form variables, it's possible that those values can be unmarshalled as floating point numbers. Which would be incompatible with the current org ID's. This change asserts that the type of number for rhc_organization should be an integer.

Result:

Issue Tracker Tickets (Jira or BZ if any):
Jira: https://issues.redhat.com/browse/OSPRH-15097

@ptoscano
Copy link
Collaborator

All values for rhc_organization should be interger values.

This is not correct: the organization ID is defined as string in the registration server:
https://github.com/candlepin/candlepin/blob/d519490f437bda6708b7544d298fec124790ea09/api/candlepin-api-spec.yaml#L9662-L9700
(see the "id" property in particular)
It is handled as such in the client tooling (i.e. subscription-manager, rhc), which use both the user input and the API responses as-is.

It looks like an integer now, yes, however there is no requirement for it to be so. It is also documented to be a string both in the README.md of this repository and in the RHEL documentation:
https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/automating_system_administration_by_using_rhel_system_roles/using-the-rhc-system-role-to-register-the-system_automating-system-administration-by-using-rhel-system-roles

@bshephar
Copy link
Author

All values for rhc_organization should be interger values.

This is not correct: the organization ID is defined as string in the registration server: https://github.com/candlepin/candlepin/blob/d519490f437bda6708b7544d298fec124790ea09/api/candlepin-api-spec.yaml#L9662-L9700 (see the "id" property in particular) It is handled as such in the client tooling (i.e. subscription-manager, rhc), which use both the user input and the API responses as-is.

It looks like an integer now, yes, however there is no requirement for it to be so. It is also documented to be a string both in the README.md of this repository and in the RHEL documentation: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/automating_system_administration_by_using_rhel_system_roles/using-the-rhc-system-role-to-register-the-system_automating-system-administration-by-using-rhel-system-roles

Ahh, thanks for pointing out the flaw in my assumption. We can | string instead to ensure type which would resolve the underlying issue we have. I'll change the commit and we can debate the merits of that approach further as required.

In situations where we're unmarshaling data from json to form
variables, it's possible that those values can be unmarshalled as floating
point numbers. Which would be incompatible with the current org ID's. This
change asserts that the type value of rhc_organization should be a string.

Jira: https://issues.redhat.com/browse/OSPRH-15097
Signed-off-by: Brendan Shephard <[email protected]>
@bshephar
Copy link
Author

Actually, no, that doesn't work either because then we'll just end up with 12345678.0 turning into "12345678.0". So, we need to assert the int for the .0 to be dropped.

@ptoscano
Copy link
Collaborator

We can | string instead to ensure type which would resolve the underlying issue we have.

... but why? Many of the parameters of the role are strings, and I do not see why rhc_organization is "more special" than the others. I did read the linked issue, and from my POV it feels a mistake of whatever handles the credentials (that end up also as rhc_organization) that handles the org ID as non-string at some point. Especially...

then we'll just end up with 12345678.0 turning into "12345678.0". So, we need to assert the int for the .0 to be dropped.

... because I find this as clear issue of whatever fills rhc_organization. Please fix that instead.

@bshephar
Copy link
Author

Yeah, that's fair enough. The problem is that we have an interface to allow users to provide arbitrary ansible variables, it just so happens to also be the interface for subscription-manager. Since the inputs are arbitrary, we can't unmarshal them with Go into a struct with fixed types, so we ultimately end up with floating point numbers for all numbers as a symptom. But that is really our problem, not yours.

I can fix for that by ensuring we call your role with the expected value. Cheers for the discussion

@bshephar bshephar closed this Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants