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

Optional Bool Fields Omitted During v1alpha7 → v1beta1 Conversion #2408

Open
okozachenko1203 opened this issue Jan 31, 2025 · 1 comment
Open
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@okozachenko1203
Copy link
Contributor

okozachenko1203 commented Jan 31, 2025

/kind bug

What steps did you take and what happened:
When an OpenStackCluster CR is automatically converted from v1alpha7 to v1beta1, the field DisableAPIServerFloatingIP is converted using Convert_bool_To_optional_Bool. This function omits fields that are set to false or nil, leading to their removal from the resulting CR.

if err := optional.Convert_bool_To_optional_Bool(&in.DisableAPIServerFloatingIP, &out.DisableAPIServerFloatingIP, s); err != nil {

func Convert_bool_To_optional_Bool(in *bool, out *Bool, _ conversion.Scope) error {
if !*in {
*out = nil
} else {
*out = in
}
return nil
}

If spec.disableAPIServerFloatingIP field was explicitly set as false in v1alpha7, it gets removed during the upgrade to v1beta1.
Since the OpenStackCluster validation webhook does not allow changes to this field after creation, it cannot be set back to false once omitted.
This affects 3rd party controllers/operators managing CAPO CRs, as they fail when attempting to ensure the missing field.

Other optional boolean fields converted in a similar manner are also likely impacted.

What did you expect to happen:
Boolean fields that explicitly have a value (even false) should not be omitted during conversion.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Cluster API Provider OpenStack version (Or git rev-parse HEAD if manually built): 0.11.2
  • Cluster-API version: 0.8.4
  • OpenStack version: All maintained versions
  • Minikube/KIND version:
  • Kubernetes version (use kubectl version): 1.31
  • OS (e.g. from /etc/os-release): Ubuntu Jammy
@mnaser
Copy link
Contributor

mnaser commented Jan 31, 2025

I wonder if a big part of this is probably for the fact these are "negative" boolean fields. For example, if we had a field called enableAPIServerFloatingIP and this was used, it would be just fine, cause false and no value would both result in it being disabled (implicit false), but since this is a "negative bool", this is problematic.

We'd need to audit any negative bools in thsi case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
Status: Inbox
Development

No branches or pull requests

3 participants