feat: unify worflows api, mapper and fuzzer #5304
Conversation
|
issue #5303 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| // If not provided, data associated with the workflow will not be | ||
| // CMEK-encrypted. | ||
| // +optional | ||
| KMSCryptoKeyRef *refs.KMSCryptoKeyRef `json:"kmsCryptoKeyRef,omitempty"` |
There was a problem hiding this comment.
We need to fix the json name otherwise mapper-generator cannot find out the KRM field.
There was a problem hiding this comment.
I thought the trick was that we could match by the field name, regardless of the json value. Do we have a standard for this (e.g. is it networkRef or computeNetworkRef)?
| } | ||
| } | ||
|
|
||
| if normalizer, ok := v.(refs.ExternalNormalizer); ok { |
There was a problem hiding this comment.
This is for those refs who implements ExternalNormalizer, we can have another one for the new Ref (which support FromIdentity). Will do in follow up PR.
There was a problem hiding this comment.
Which is the ref? I think ExternalNormalizer is the legacy one, and I agree this code is relatively brittle
| return nil, fmt.Errorf("error converting to %T: %w", obj, err) | ||
| } | ||
|
|
||
| if err := common.NormalizeReferences(ctx, reader, obj, nil); err != nil { |
BRIEF Change description
Fixes #
WHY do we need this change?
Special notes for your reviewer:
Does this PR add something which needs to be 'release noted'?
Additional documentation e.g., references, usage docs, etc.:
Intended Milestone
Please indicate the intended milestone.
Tests you have done
make ready-prto ensure this PR is ready for review.