- Notifications
You must be signed in to change notification settings - Fork923
chore: implement yaml parsing for external auth configs#11268
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
githubCfg := codersdk.ExternalAuthConfig{ | ||
Type: "github", | ||
ClientID: "client_id", | ||
ClientSecret: "client_secret", | ||
ID: "id", | ||
AuthURL: "https://example.com/auth", | ||
TokenURL: "https://example.com/token", | ||
ValidateURL: "https://example.com/validate", | ||
AppInstallURL: "https://example.com/install", | ||
AppInstallationsURL: "https://example.com/installations", | ||
NoRefresh: true, | ||
Scopes: []string{"user:email", "read:org"}, | ||
ExtraTokenKeys: []string{"extra", "token"}, | ||
DeviceFlow: true, | ||
DeviceCodeURL: "https://example.com/device", | ||
Regex: "^https://example.com/.*$", | ||
DisplayName: "GitHub", | ||
DisplayIcon: "/static/icons/github.svg", | ||
} |
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.
Instead of hard-coding this and having to keep it in sync with the golden file, how about marshalling and unmarshalling the config to/from YAML and asserting that the two are the same byte-for-byte?
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 can do both, but I should still have the golden file imo. The actual struct isclibase.Struct[[]ExternalAuthConfig]
. So it might unmarshal/marshal, but the nested key structure could be messed up. We want to make sure the actual Go slice type is at the right yaml level (top level) imo.
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.
This looks OK to me, but I'm by no means the authority on using YAML config.
Uh oh!
There was an error while loading.Please reload this page.
What this does
Allows user to configure external auths from a yaml file.