Add cf-metadata parameter for user-defined CF labels and annotations#1879
Add cf-metadata parameter for user-defined CF labels and annotations#1879Yavor16 wants to merge 5 commits into
Conversation
Introduces the cf-metadata module/resource parameter allowing users to attach user-defined CF V3 labels and annotations via mtad.yaml. Adds CfUserMetadata value object, CfMetadataParser, CfMetadataValidator with reserved-key and format enforcement. Merges user labels/annotations into ApplicationMetadataBuilder and ServiceMetadataBuilder. Threads userCfMetadata through CloudApplicationExtended and CloudServiceInstanceExtended. Changes metadata change-detection in CreateOrUpdateAppStep and DetermineServiceCreateUpdateServiceActionsStep to compare only user-defined keys, preventing MTA-internal churn from triggering spurious CF metadata API calls. JIRA:LMCROSSITXSADEPLOY-1550
…MetadataBuilder Fix RESERVED_KEY_NAMES in CfMetadataValidator to use HashSet construction instead of Set.of() to tolerate duplicate string values that appear in both MtaMetadataLabels and MtaMetadataAnnotations (e.g. "mta_id", "mta_namespace"). JIRA:LMCROSSITXSADEPLOY-1550
- ApplicationCloudModelBuilderCfMetadataTest: end-to-end test that cf-metadata in module parameters flows to getUserCfMetadata() on the built app - CreateOrUpdateStepWithExistingAppTest: three tests verifying that user-label changes trigger updateApplicationMetadata, while MTA-internal-only changes do not - DetermineServiceCreateUpdateServiceActionsStepTest: three tests verifying that user-label changes trigger UPDATE_METADATA action for services - Fix filterUserKeys NPE when Metadata.getLabels()/getAnnotations() returns null (CF client v3 Metadata can return null for absent maps) JIRA:LMCROSSITXSADEPLOY-1550
…ites JIRA:LMCROSSITXSADEPLOY-1550
Yavor16
left a comment
Yavor16
left a comment
There was a problem hiding this comment.
Automated code review by MTA Quality Agent — 1 finding.
MTA Quality Report — cloudfoundry/multiapps-controller PR #1879Jira: LMCROSSITXSADEPLOY-1550 — (backlog item could not be fetched in this environment)
Code Review6 finding(s): 1 posted inline · 5 general (see below). Security
SonarCloud
Quality gate: UNKNOWN The Dependency CVEs✅ No new CVEs — this PR does not modify any dependency files ( General findings
|
The prior alternation was at the top level of the regex, so the single-character alternative only matched bare unprefixed keys. A key like foo.com/x (valid per CF) was incorrectly rejected because the name-segment after the slash required at least two characters. Wrapping the name-segment alternatives in a non-capturing group ensures both the multi-char and single-char alternatives apply to bare keys and to the name segment of prefixed keys equally. JIRA:LMCROSSITXSADEPLOY-1550
Yavor16
left a comment
Yavor16
left a comment
There was a problem hiding this comment.
Automated code review by MTA Quality Agent — 2 finding(s).
| public class CfMetadataValidator { | ||
|
|
||
| private static final Pattern KEY_PATTERN = Pattern.compile( | ||
| "([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,251}[a-zA-Z0-9]/)?([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,61}[a-zA-Z0-9]|[a-zA-Z0-9])"); |
There was a problem hiding this comment.
Severity: IMPORTANT · Confidence: 85
The prefix sub-pattern [a-zA-Z0-9][a-zA-Z0-9\-_.]{0,251}[a-zA-Z0-9] requires a minimum of two characters, so a single-character DNS prefix such as a/mykey is incorrectly rejected as invalid. RFC 1123 allows single-character DNS labels, and the error message does not document a two-character minimum for the prefix, so users will receive a misleading "invalid key" error for valid keys.
Evidence: "([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,251}[a-zA-Z0-9]/)?([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,61}[a-zA-Z0-9]|[a-zA-Z0-9])");
Fix:
| "([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,251}[a-zA-Z0-9]/)?([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,61}[a-zA-Z0-9]|[a-zA-Z0-9])"); | |
| "([a-zA-Z0-9]([a-zA-Z0-9\\-_.]{0,251}[a-zA-Z0-9])?/)?([a-zA-Z0-9][a-zA-Z0-9\\-_.]{0,61}[a-zA-Z0-9]|[a-zA-Z0-9])"); |
| return; | ||
| } | ||
| if (!Objects.equals(existingApp.getV3Metadata(), app.getV3Metadata())) { | ||
| if (hasUserMetadataChanged(existingApp.getV3Metadata(), app.getV3Metadata())) { |
There was a problem hiding this comment.
Severity: IMPORTANT · Confidence: 82
The sets internalLabelKeys and internalAnnotationKeys are defined as inline literals inside hasUserMetadataChanged, duplicating the identical definitions in DetermineServiceCreateUpdateServiceActionsStep.shouldUpdateMetadata. Any future addition of a new internal metadata key (or removal of an obsolete one) must be applied in both places; missing one silently changes the change-detection semantics for either apps or services. Both sets should be extracted to shared constants.
Evidence: Set<String> internalLabelKeys = Set.of(MtaMetadataLabels.MTA_ID, MtaMetadataLabels.MTA_NAMESPACE, MtaMetadataLabels.SPACE_GUID,
Fix: Extract both internalLabelKeys and internalAnnotationKeys as private static final Set<String> constants in a shared location (e.g. MtaMetadataLabels/MtaMetadataAnnotations or a new MtaInternalMetadataKeys class), and reuse them here, in DetermineServiceCreateUpdateServiceActionsStep, and in CfMetadataValidator.RESERVED_KEY_NAMES.
MTA Quality Report — cloudfoundry/multiapps-controller PR #1879Jira: LMCROSSITXSADEPLOY-1550 — (backlog item could not be fetched in this environment; the
Code Review2 finding(s): 2 posted inline. Security
SonarCloud
Quality gate: UNKNOWN The SonarCloud-associated job ("Build and analyze") failed at the build/workflow level before producing a quality-gate result. No SonarCloud quality-gate conclusion or per-line issue annotations were emitted. FindingsNo SonarCloud per-line code smells, bugs, or vulnerabilities were surfaced as check-run annotations. Dependency CVEs⏭️ CVE scan skipped on this review-fix re-run — dependencies were scanned on the initial quality run. |
1 participant
What
Adds support for a new
cf-metadataMTA parameter that lets descriptor authorsattach user-defined Cloud Foundry V3 labels and annotations to applications and
managed service instances directly from
mtad.yaml.Why
Previously, user-defined CF metadata (labels and annotations) could only be set
manually after deployment, which was error-prone and not reproducible from the
MTA descriptor. This change makes that metadata declarative and part of the
deployment lifecycle.
How
cf-metadatamodule/resource parameter registered inSupportedParameters.CfMetadataParserparses thelabelsandannotationssub-maps into a newCfUserMetadatavalue object.CfMetadataValidatorenforces CF metadata key/value constraints and rejectsreserved
mta.cloudfoundry.org/keys with a clear error.CloudApplicationExtendedandCloudServiceInstanceExtended, and merged into the builtMetadatabyApplicationMetadataBuilderandServiceMetadataBuilder.CreateOrUpdateAppStepandDetermineServiceCreateUpdateServiceActionsStepcompares only user-definedkeys, so updates to MTA-internal metadata no longer trigger spurious CF
metadata API calls, and user metadata changes correctly trigger an update.
Behaviour notes
mta.cloudfoundry.org/labels and annotations are alwayspreserved; user descriptors cannot overwrite or delete them.
interpolation.
cf-metadatadeploy identically to prior behaviour.Tests
CfMetadataParserTest,CfMetadataValidatorTestApplicationMetadataBuilderTest,ServiceMetadataBuilderTest,ApplicationCloudModelBuilderCfMetadataTestCreateOrUpdateStepWithExistingAppTest,DetermineServiceCreateUpdateServiceActionsStepTestJIRA:LMCROSSITXSADEPLOY-1550