-
Notifications
You must be signed in to change notification settings - Fork 228
DRAFT: add ARO HCP - redhatopenshift/v1api20240610preview #4964
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
base: main
Are you sure you want to change the base?
DRAFT: add ARO HCP - redhatopenshift/v1api20240610preview #4964
Conversation
7d5a7cb to
3056677
Compare
theunrepentantgeek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good start - looking forward to seeing this merged.
Your next step will be to write a test to exercise the new resource and confirm that it works as intended.
Despite ARM providing a uniform API, we've found enough variation in the way different resources behave that we've needed to create a number of extension-points for customization. A coded test in the controllers package is one good way to flush out any issues in this areas.
Once that test passes, your samples need testing too.
A quick security note - all the test recordings are automatically redacted, removing sensitive information such as subscription IDs, access tokens, SSH keys and so on.
| 2024-06-10-preview: | ||
| HcpOpenShiftClusters_NodePool: | ||
| $supportedFrom: v2.16.0 | ||
| $exportAs: HcpOpenShiftClustersNodePool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the group redhatopenshift will always be present when the resource is specified - effectively scoping the resource name - I'd suggest simplifying all the resource names. They don't need to have a common prefix.
| $exportAs: HcpOpenShiftClustersNodePool | |
| $exportAs: OpenShiftClusterNodePool |
or maybe even
| $exportAs: HcpOpenShiftClustersNodePool | |
| $exportAs: NodePool |
| $exportAs: HcpOpenShiftClustersNodePool | ||
| HcpOpenShiftClusters_ExternalAuth: | ||
| $supportedFrom: v2.16.0 | ||
| $exportAs: HcpOpenShiftClustersExternalAuth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $exportAs: HcpOpenShiftClustersExternalAuth | |
| $exportAs: ExternalAuth |
| │ │ │ ├── Rule 0: Maximum: 10080 | ||
| │ │ │ └── Rule 1: Minimum: 0 | ||
| │ │ ├── Platform: *Object (5 properties) | ||
| │ │ │ ├── ManagedResourceGroup: *string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a simple string (e.g. my-rg) or is it an ARM reference (/subscriptions/resourcegroups/my-rg)? If the later, you should mark this with $referenceType: arm in azure-arm.yaml.
Followup question - can this be user specified? If not, you may need to use a TypeTransformer to trim it from the Spec type. There are plenty of examples on how to do this already azure-arm.yaml.
|
We're planning on releasing ASO v2.16 next week (roughly ETA Oct 13). Do you think that you can respond to @theunrepentantgeek's comments by then @marek-veber? Let us know if you'll not be able to make that but think you could make slightly later that week as there may a little flex possible in the schedule. |
3056677 to
3807781
Compare
theunrepentantgeek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making good progress.
Do you need to update the submodule?
It's usual for updates of the azure-rest-api-specs submodule to bring along documentation changes and other things - and since our CI checks that generated code is up to date, this might fail the builds.
Please check to see if the generated code for any other resources is changed by this - if so, let me know and I'll do an isolated update PR for the submodule to pull in those changes without polluting your PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We won't be able to merge the PR while these files are present; best to rebase and omit the commit that added them.
|
@marek-veber Thanks for this PR, as we want to have the same..! |
Add ARO (Azure RedHat OpenShift) HCP support
This PR include support for Microsoft.RedHatOpenShift HCP (Hosted Control Plane) clusters with API version 2024-06-10-preview.
Changes Made
HcpOpenShiftClusters- Main HCP cluster resourceNodePools- Node pool managementExternalAuths- External authentication configurationSpecial Notes
This update brings in the latest Azure REST API specifications that include Azure RedHat OpenShift HCP support. The new API version
2024-06-10-previewprovides preview functionality for managing hosted control plane OpenShift clusters on Azure.Checklist