feat: Enhance UUID validation with subaccount ID support#230
feat: Enhance UUID validation with subaccount ID support#230
Conversation
Refactor UUID validation to include SAP BTP short subaccount ID format.
|
|
vipinvkmenon
changed the title
|
@davirezendegb Can you sign the CLA agreement? This is a prerequisite for us to accept contributions. Please also check that the build can be executed successfully (see https://github.com/SAP/terraform-provider-scc/actions/runs/21529053233/job/62243710758?pr=230) and that the documentation is up to date (i.e. execute |
There was a problem hiding this comment.
Pull request overview
Refactors the UUID validator to also accept SAP BTP “short” subaccount IDs so SAP-managed subaccounts without full UUIDs can be used.
Changes:
- Expands the
UuidRegexppattern to match either a standard UUID or a short subaccount ID ([a-z][0-9a-fA-F]{8}). - Updates the validator error message to mention the additional accepted format.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // UuidRegexp matches: | ||
| // - Standard UUID format: 8-4-4-4-12 (hex with hyphens) | ||
| // - SAP subaccount ID format: [a-z] + 8 hex characters (e.g. xf014edd7) | ||
| var UuidRegexp = regexp.MustCompile( | ||
| `^(?:` + | ||
| `[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}` + | ||
| `|` + | ||
| `[a-z][0-9a-fA-F]{8}` + | ||
| `)$`, |
There was a problem hiding this comment.
New behavior adds acceptance of the SAP short subaccount ID pattern ([a-z][0-9a-fA-F]{8}), but the package unit tests currently only cover standard UUIDs and a generic mismatch. Add explicit test cases for a valid short subaccount ID and a few invalid variants (wrong length / invalid hex) to prevent regressions.
| // ValidUUID validates that the string attribute value is either | ||
| // a standard UUID or a SAP subaccount ID | ||
| func ValidUUID() validator.String { |
There was a problem hiding this comment.
ValidUUID (and UuidRegexp) now validates more than UUIDs (it also accepts SAP short subaccount IDs). To avoid confusion for future uses, consider introducing a more accurately named validator (e.g., ValidSubaccountID / ValidSubaccountIdentifier) and either migrate call sites or keep ValidUUID as a backwards-compatible alias.
| // ValidUUID validates that the string attribute value is either | ||
| // a standard UUID or a SAP subaccount ID | ||
| func ValidUUID() validator.String { | ||
| return stringvalidator.RegexMatches(UuidRegexp, "value must be a valid UUID") | ||
| return stringvalidator.RegexMatches( | ||
| UuidRegexp, | ||
| "value must be a valid UUID or SAP subaccount id ([a-z]########)", | ||
| ) |
There was a problem hiding this comment.
The error message/format hint doesn’t match the actual regex: ([a-z]########) suggests 1 letter + 8 digits, but the validator accepts 1 lowercase letter + 8 hex chars ([0-9a-fA-F]{8}). Also consider consistent capitalization/grammar (e.g., “SAP subaccount ID” and “an SAP subaccount ID”). Update the message (and/or comment) so users get an accurate validation hint.
|
Merging the PR, changes requested by copilot will be part of another PR. #232 |
|
@davirezendegb Thanks for your contribution. The change will be part of the next release of this provider |
3 participants
Refactor UUID validation to include SAP BTP short subaccount ID format.
Could you make the correction? Because there are sub-accounts that are managed by SAP and do not contain a large UUID.