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

feat(bundle): ENTESB-15469 - upgrade CRD api version from v1beta to v1 to support default values in schema #46

Merged
merged 1 commit into from Jan 22, 2021

Conversation

tadayosi
Copy link
Member

https://issues.redhat.com/browse/ENTESB-15469

Since apiextensions.k8s.io/v1 CRD supports default values in openAPIV3Schema and OpenShift web console just relies on react-jsonschema-form for the operand form view (which supports the schema default values as well), I think it's good idea to bump up the version of CRD to apiextensions.k8s.io/v1 and introduce a default value for the RBAC enabled property as the more permanent solution to ENTESB-15469.

@astefanutti Please review and let me know your thoughts.

@astefanutti
Copy link
Contributor

@tadayosi thanks a lot. Please find my feedback:

  • Upgrading to apiextensions.k8s.io/v1 is very important as v1beta1 is going to be removed soon. The only concern is that we would possibly want to support OpenShift 3 for downstream projects, for which v1 is not available. So that's just for us to keep in mind that at some point, we may have to provide a v1beta1 version of the CRD back, but I don't see any trouble here, as the CRD is quite simple.
  • Now for the rbac.enabled field, may I propose an alternative solution, which is to remove it :) I introduced it as a sort of feature toggle, when the RBAC development came in, to make sure users have an option to turn it off if something goes wrong, but now that it's been around for some time and I don't see any reason why users would want to turn RBAC support off, I'd be inclined to remove the field, to simplify the configuration surface.

@tadayosi
Copy link
Member Author

@astefanutti Thanks for your comment. OK, I overlooked backward compatibility for OpenShift 3. It seems to be important to maintain for a while. This pull req doesn't add much value to hawtio-operator other than improving ENTESB-15469 at this moment, so let's leave it for now.

I agree with your alternative solution. I'll work on it instead soon.

@astefanutti
Copy link
Contributor

We can do the upgrade to apiextensions.k8s.io/v1 as I don't think the operator is used on OpenShift 4. On OpenShift 3, which is pre-operator era, users rely on the templates. But there has been discussions to get rid of these and propose the operator as the only way to install the console. In that case, will have to figure out a way to offer apiextensions.k8s.io/v1beta1 version of the CRD. But until then, it's I think we can move forward and upgrade to apiextensions.k8s.io/v1.

@tadayosi
Copy link
Member Author

OK, after the other pull is merged I'll revisit to work on the migration from v1beta to v1 here again.

@tadayosi
Copy link
Member Author

@astefanutti Updated. Please check it again. Thanks.

@astefanutti
Copy link
Contributor

We should keep in mind the upgrade use case, just to make sure it goes smoothly before we release a new version.

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.

None yet

2 participants