-
Notifications
You must be signed in to change notification settings - Fork 4
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
PoC - Logging, Authentication, generated clients for IAM, Secrets, Cosmos #8
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.
Looks awesome! I've just left random thoughts that don't have to be settled in this PR necessarily.
Can you re-run CI? As it now runs golint we should make sure it passes before merging.
@@ -0,0 +1,938 @@ | |||
swagger: '2.0' |
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.
Can we have the spec definitions in an openapi
directory? As it's the new name for them.
}, nil | ||
} | ||
|
||
func (c *Client) Login(username, password string) (string, error) { |
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.
Given that we have different login methods (username/password, service account, oidc/saml), in the future either we say uid/password is the default and keep it named Login
, then we'll add eg. ServiceLogin
, we could also make the arguments more generic instead.
@@ -0,0 +1,27 @@ | |||
#!/usr/bin/env bash |
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.
Ideally this would be triggered by go generate
Revert changes in generated code
The underlying `*http.Client` that we provide already sets the header
d3c7f38
to
61e6f40
Compare
Rebased and pushed. CI now runs and fails due to missing comments. But I have to say, since it's just a proof of concept, I really don't want to merge this as it is 😉 It should rather serve as an example to see what's possible and how it would be possible and what could be done in the future |
This is just a proof-of-concept for several requirements that we're after:
The most important file to look at is
example.org
which contains examples forNoteworthy:
PackageOptions
struct has been manually created inside thedcos/cosmos
folder (which means we are able to manually create helper methods inside the sub-clients)The code is not clean, it's not tested, there is duplication, and it could and should be moved around a little bit, but it allows us to see what's possible :)