Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions api/hypershift/v1beta1/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,12 @@ type AzureVMImage struct {
Type AzureVMImageType `json:"type"`

// imageID is the Azure resource ID of a VHD image to use to boot the Azure VMs from.
// TODO: What is the valid character set for this field? What about minimum and maximum lengths?
// The expected format is an Azure resource ID string. This can be a managed image or an
// Azure Compute Gallery image version, for example:
// /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/images/{imageName}
// /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinitionName}/versions/{imageVersionName}
// See https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules for
// Azure resource naming rules and restrictions.
//
// +optional
// +unionMember
Expand Down Expand Up @@ -166,9 +171,11 @@ type AzureMarketplaceImage struct {
ImageGeneration *AzureVMImageGeneration `json:"imageGeneration,omitempty"`

// publisher is the name of the organization that created the image.
// It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, and hyphens (-) and underscores (_).
// For example, "azureopenshift", "canonical", or "redhat".
// It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, hyphens (-), and underscores (_).
// It must start with a lowercase letter or a number.
Comment on lines +174 to 176

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would something like publisher10------__----- actually be valid?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, publisher10------__----- would pass this regex. The validation here is intentionally scoped to character set and length — it ensures only the documented characters (lowercase alphanumeric, hyphens, underscores) are used and the value starts with a letter or digit, but it doesn't enforce restrictions on consecutive special characters or trailing patterns.

The rationale is that Azure's Compute API will reject any publisher name that doesn't match an actual registered publisher at deployment time, so the CRD validation acts as a first-pass format check rather than a semantic one. Tightening the regex (e.g., disallowing consecutive hyphens/underscores) risks rejecting valid Azure publisher names we haven't encountered, since Azure's exact naming rules aren't fully documented.

That said, if you'd prefer stricter validation here (e.g., requiring special chars to be non-consecutive or non-trailing), happy to tighten it — just want to be cautious about false negatives.


AI-assisted response via Claude Code

// TODO: Can we explain where a user might find this value, or provide an example of one they might want to use
// See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
// information on Azure Marketplace image publishers.
//
// +kubebuilder:validation:Pattern=`^[a-z0-9][a-z0-9-_]{2,49}$`
// +kubebuilder:validation:MinLength=3
Expand All @@ -177,17 +184,25 @@ type AzureMarketplaceImage struct {
Publisher string `json:"publisher,omitempty"`

// offer specifies the name of a group of related images created by the publisher.
// TODO: What is the valid character set for this field? What about minimum and maximum lengths?
//
// For example, "RHEL", "WindowsServer", or "0001-com-ubuntu-server-jammy".
// The value must consist of only alphanumeric characters (a-z, A-Z, 0-9),
// hyphens (-), underscores (_), and periods (.).
// It must start with an alphanumeric character.
Comment on lines +188 to +190

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would something like offer-thing----___---..... actually be valid?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, offer-thing----___---..... would pass this regex. Same reasoning as above — the validation is scoped to allowed character set (alphanumeric, hyphens, underscores, periods) and requiring an alphanumeric first character, without restricting consecutive or trailing special characters.

Real-world Azure offer names vary widely in format (e.g., 0001-com-ubuntu-server-jammy, RHEL, WindowsServer), and the periods in particular appear in some offer names. Since Azure's exact naming constraints for offers aren't exhaustively documented, the regex errs on the permissive side to avoid rejecting valid values. Azure's API will return a clear error at deployment time for any non-existent offer.

If you'd like stricter rules (e.g., no consecutive special chars, must end with alphanumeric), happy to adjust — just flagging the trade-off with potential false negatives.


AI-assisted response via Claude Code

// See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a canonical source to API rather than CLI?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The closest canonical API source is the Azure Compute REST API reference for Virtual Machine Images:
https://learn.microsoft.com/en-us/rest/api/compute/virtual-machine-images

With specific operations for each field:

However, the REST API docs don't document the character set constraints explicitly — those come from the Partner Center docs for publishers creating offers (e.g., https://learn.microsoft.com/en-us/partner-center/marketplace-offers/azure-vm-offer-setup). The CLI page currently linked is arguably the most user-friendly consumer-facing reference for finding valid values.

Happy to update the links to the REST API reference or the Partner Center docs — let me know which you'd prefer.


AI-assisted response via Claude Code

// information on Azure Marketplace image offers.
//
// +kubebuilder:validation:Pattern=`^[a-zA-Z0-9][a-zA-Z0-9._-]*$`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please use // +kubebuilder:validation:XValidation:rule="self.matches and add an envtest to validate this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Replaced +kubebuilder:validation:Pattern with +kubebuilder:validation:XValidation:rule="self.matches('^[a-zA-Z0-9][a-zA-Z0-9._-]*$')" for the offer field, consistent with the pattern used elsewhere in the codebase (e.g., GCP project IDs in gcp.go, Azure client IDs in azure.go).

Added three envtest cases to stable.nodepools.azure.testsuite.yaml:

  • Rejects offer with invalid characters (spaces, !)
  • Rejects offer starting with a hyphen
  • Accepts offer with valid mixed characters (alphanumeric, hyphens, underscores, periods)

Regenerated CRDs and vendor.


AI-assisted response via Claude Code

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=255
// +optional
Offer string `json:"offer,omitempty"`

// sku specifies an instance of an offer, such as a major release of a distribution.
// For example, 22_04-lts-gen2, 8-lvm-gen2.
// The value must consist only of lowercase letters, numbers, and hyphens (-) and underscores (_).
// TODO: What about length limits?
// The value must be between 1 and 255 characters in length, and consist of only lowercase
// letters, numbers, hyphens (-), and underscores (_).
Comment on lines +202 to +203

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would something like __---_------------sku123 actually be valid?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, __---_------------sku123 would pass this regex (^[a-z0-9-_]+$). Notably, unlike the publisher and offer fields, the SKU regex doesn't require starting with an alphanumeric character — it allows hyphens and underscores anywhere, including as the first character.

Real-world SKU values like 22_04-lts-gen2 and 8-lvm-gen2 do use hyphens and underscores freely. However, looking at this more carefully, the documentation on this PR says "The value must consist of only lowercase letters, numbers, hyphens (-), and underscores (_)" without explicitly saying it must start with alphanumeric. If you think the SKU field should also require starting with an alphanumeric character (consistent with the publisher field), the regex could be tightened to ^[a-z0-9][a-z0-9-_]*$. That would reject values starting with _ or -.

Let me know if you'd like that tightened, or if the current permissive approach is acceptable given that Azure's API provides the ultimate validation.


AI-assisted response via Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, __---_------------sku123 would technically pass the current regex (^[a-z0-9-_]+$). Unlike the publisher and offer fields, the SKU regex doesn't require starting with an alphanumeric character — so values beginning with _ or - are accepted.

In practice, real Azure SKUs (e.g. 22_04-lts-gen2, 8-lvm-gen2, 2022-datacenter) always start with an alphanumeric character. If you'd like, we can tighten this to ^[a-z0-9][a-z0-9-_]*$ to match that pattern consistently with the publisher field. Let me know if you'd like that change.


AI-assisted response via Claude Code

// See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
// information on Azure Marketplace image SKUs.
//
// +kubebuilder:validation:Pattern=`^[a-z0-9-_]+$`
// +kubebuilder:validation:MinLength=1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,16 +837,26 @@ spec:
- Gen2
type: string
offer:
description: offer specifies the name of a group of
related images created by the publisher.
description: |-
offer specifies the name of a group of related images created by the publisher.
For example, "RHEL", "WindowsServer", or "0001-com-ubuntu-server-jammy".
The value must consist of only alphanumeric characters (a-z, A-Z, 0-9),
hyphens (-), underscores (_), and periods (.).
It must start with an alphanumeric character.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image offers.
maxLength: 255
minLength: 1
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]*$
type: string
publisher:
description: |-
publisher is the name of the organization that created the image.
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, and hyphens (-) and underscores (_).
For example, "azureopenshift", "canonical", or "redhat".
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, hyphens (-), and underscores (_).
It must start with a lowercase letter or a number.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image publishers.
maxLength: 50
minLength: 3
pattern: ^[a-z0-9][a-z0-9-_]{2,49}$
Expand All @@ -855,7 +865,10 @@ spec:
description: |-
sku specifies an instance of an offer, such as a major release of a distribution.
For example, 22_04-lts-gen2, 8-lvm-gen2.
The value must consist only of lowercase letters, numbers, and hyphens (-) and underscores (_).
The value must be between 1 and 255 characters in length, and consist of only lowercase
letters, numbers, hyphens (-), and underscores (_).
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image SKUs.
maxLength: 255
minLength: 1
pattern: ^[a-z0-9-_]+$
Expand Down Expand Up @@ -884,8 +897,14 @@ spec:
has(self.version)].filter(x, x == true).size() ==
4'
imageID:
description: imageID is the Azure resource ID of a VHD
image to use to boot the Azure VMs from.
description: |-
imageID is the Azure resource ID of a VHD image to use to boot the Azure VMs from.
The expected format is an Azure resource ID string. This can be a managed image or an
Azure Compute Gallery image version, for example:
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/images/{imageName}
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinitionName}/versions/{imageVersionName}
See https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules for
Azure resource naming rules and restrictions.
maxLength: 255
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,16 +837,26 @@ spec:
- Gen2
type: string
offer:
description: offer specifies the name of a group of
related images created by the publisher.
description: |-
offer specifies the name of a group of related images created by the publisher.
For example, "RHEL", "WindowsServer", or "0001-com-ubuntu-server-jammy".
The value must consist of only alphanumeric characters (a-z, A-Z, 0-9),
hyphens (-), underscores (_), and periods (.).
It must start with an alphanumeric character.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image offers.
maxLength: 255
minLength: 1
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]*$
type: string
publisher:
description: |-
publisher is the name of the organization that created the image.
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, and hyphens (-) and underscores (_).
For example, "azureopenshift", "canonical", or "redhat".
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, hyphens (-), and underscores (_).
It must start with a lowercase letter or a number.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image publishers.
maxLength: 50
minLength: 3
pattern: ^[a-z0-9][a-z0-9-_]{2,49}$
Expand All @@ -855,7 +865,10 @@ spec:
description: |-
sku specifies an instance of an offer, such as a major release of a distribution.
For example, 22_04-lts-gen2, 8-lvm-gen2.
The value must consist only of lowercase letters, numbers, and hyphens (-) and underscores (_).
The value must be between 1 and 255 characters in length, and consist of only lowercase
letters, numbers, hyphens (-), and underscores (_).
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image SKUs.
maxLength: 255
minLength: 1
pattern: ^[a-z0-9-_]+$
Expand Down Expand Up @@ -884,8 +897,14 @@ spec:
has(self.version)].filter(x, x == true).size() ==
4'
imageID:
description: imageID is the Azure resource ID of a VHD
image to use to boot the Azure VMs from.
description: |-
imageID is the Azure resource ID of a VHD image to use to boot the Azure VMs from.
The expected format is an Azure resource ID string. This can be a managed image or an
Azure Compute Gallery image version, for example:
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/images/{imageName}
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinitionName}/versions/{imageVersionName}
See https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules for
Azure resource naming rules and restrictions.
maxLength: 255
type: string
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,16 +837,26 @@ spec:
- Gen2
type: string
offer:
description: offer specifies the name of a group of
related images created by the publisher.
description: |-
offer specifies the name of a group of related images created by the publisher.
For example, "RHEL", "WindowsServer", or "0001-com-ubuntu-server-jammy".
The value must consist of only alphanumeric characters (a-z, A-Z, 0-9),
hyphens (-), underscores (_), and periods (.).
It must start with an alphanumeric character.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image offers.
maxLength: 255
minLength: 1
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]*$
type: string
publisher:
description: |-
publisher is the name of the organization that created the image.
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, and hyphens (-) and underscores (_).
For example, "azureopenshift", "canonical", or "redhat".
It must be between 3 and 50 characters in length, and consist of only lowercase letters, numbers, hyphens (-), and underscores (_).
It must start with a lowercase letter or a number.
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image publishers.
maxLength: 50
minLength: 3
pattern: ^[a-z0-9][a-z0-9-_]{2,49}$
Expand All @@ -855,7 +865,10 @@ spec:
description: |-
sku specifies an instance of an offer, such as a major release of a distribution.
For example, 22_04-lts-gen2, 8-lvm-gen2.
The value must consist only of lowercase letters, numbers, and hyphens (-) and underscores (_).
The value must be between 1 and 255 characters in length, and consist of only lowercase
letters, numbers, hyphens (-), and underscores (_).
See https://learn.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage for more
information on Azure Marketplace image SKUs.
maxLength: 255
minLength: 1
pattern: ^[a-z0-9-_]+$
Expand Down Expand Up @@ -884,8 +897,14 @@ spec:
has(self.version)].filter(x, x == true).size() ==
4'
imageID:
description: imageID is the Azure resource ID of a VHD
image to use to boot the Azure VMs from.
description: |-
imageID is the Azure resource ID of a VHD image to use to boot the Azure VMs from.
The expected format is an Azure resource ID string. This can be a managed image or an
Azure Compute Gallery image version, for example:
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/images/{imageName}
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinitionName}/versions/{imageVersionName}
See https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules for
Azure resource naming rules and restrictions.
maxLength: 255
type: string
type:
Expand Down
Loading
Loading