Skip to content
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

fix: setting secrets and configs error with aliased field names #2103

Merged

Conversation

wesbillman
Copy link
Collaborator

Fixes an error where

type SomeSecret struct {
	FirstName string `json:"first-name"`
}

when set with secret set {"first-name": "whatever"}

@wesbillman wesbillman requested a review from alecthomas as a code owner July 17, 2024 15:06
@wesbillman wesbillman requested review from a team and deniseli and removed request for a team July 17, 2024 15:06
@ftl-robot ftl-robot mentioned this pull request Jul 17, 2024
@wesbillman wesbillman force-pushed the use-transformed-alias-fields-in-secret-and-config-set branch from 77197d8 to 4514a96 Compare July 17, 2024 15:43
Comment on lines +404 to +406
`failed to transform aliased fields: 0:0: expected 'name' field to be a string, got int`},
{&schema.Ref{Name: "TypeEnumRequest", Module: "test"}, obj{"message": "Hello"},
`malformed enum type test.TypeEnumRequest.message: expected structure is {"name": "<variant name>", "value": <variant value>}`},
`failed to transform aliased fields: 0:0: expected map, got string`},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@safeer we might want to come back and fix up these error messages if possible to make them more readable

@wesbillman wesbillman merged commit 15f04ef into main Jul 17, 2024
59 checks passed
@wesbillman wesbillman deleted the use-transformed-alias-fields-in-secret-and-config-set branch July 17, 2024 16:46
@deniseli deniseli added the approved Marks an already closed PR as approved label Jul 17, 2024
@@ -211,6 +216,7 @@ func ValidateJSONValue(fieldType Type, path path, value any, sch *Schema) error
return nil
}

// ValidateRequestMap validates a given JSON map against the provided schema.
func ValidateRequestMap(ref *Ref, path path, request map[string]any, sch *Schema) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note for future reference:
Since this function is only called by ingress.BuildRequestBody and ValidateJSONValue above, we could have this function call TransformFromAliasedFields directly. Then, the caller doesn't need to know to do it, so we can prevent issues like this in the future.

Discussed on Slack - we'll leave this as is for now to keep everything as stable as possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants