-
Notifications
You must be signed in to change notification settings - Fork 887
feat: support nested structs, structured arrays, and better secret value handling in config #4727
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that this is structured similarly to the YAML... it's nice!
codersdk/deploymentconfig.go
Outdated
return json.Marshal(struct { | ||
Name string `json:"name"` | ||
Usage string `json:"usage"` | ||
Flag string `json:"flag"` | ||
Shorthand string `json:"shorthand"` | ||
Enterprise bool `json:"enterprise"` | ||
Hidden bool `json:"hidden"` | ||
Secret bool `json:"secret"` | ||
Default T `json:"default"` | ||
Value T `json:"value"` | ||
}{ | ||
Name: f.Name, | ||
Usage: f.Usage, | ||
Flag: f.Flag, | ||
Shorthand: f.Shorthand, | ||
Enterprise: f.Enterprise, | ||
Hidden: f.Hidden, | ||
Secret: f.Secret, | ||
Default: f.Default, | ||
Value: f.Value, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just marshal ourselves in this case?
json.Marshal(f)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't! I did that first and you get an infinite loop because it just calls this marshaller again. LOL
It's exactly the structure, notice there are no mapstructure tags. The json and yaml will always stay in sync this way. I believe in the future we can implement a yaml marshaller and use it to write this struct to yaml, if for some reason we don't want to use viper for that. |
What this changes:
Path
field in config, we just use the json tags nowCODER_$INDEX_$FIELD
MarshalJSON
we can make sure we never return secrets fields over the wire.