Add options to docker login azure to support Service Principal login. Use it in E2E tests
#448
Conversation
aci/backend.go
Outdated
| } | ||
|
|
||
| func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { | ||
| createOpts := params.(LoginParams) | ||
| return cs.loginService.Login(ctx, createOpts.TenantID) | ||
| loginOpts := params.(LoginParams) |
There was a problem hiding this comment.
Do we need a check here to make sure the cast was good?
We can just call this opts as we're in the login function.
| loginOpts := params.(LoginParams) | |
| opts := params.(LoginParams) |
aci/backend.go
Outdated
| loginOpts := params.(LoginParams) | ||
| if loginOpts.ClientID != "" { | ||
| if loginOpts.ClientSecret == "" || loginOpts.TenantID == "" { | ||
| return errors.New("for Service Principal login, 3 options must be specified : --client-id, --client-secret and --tenant-id") |
There was a problem hiding this comment.
nit: when we put in place cleaner errors, we should specify things like flags at the CLI level instead of in the backend.
There was a problem hiding this comment.
Yes, I wasn't sure where to put this validation code ; I've put this next to the check for clientID required anyway to call LoginServiceProvider, but that could be moved somewhere else...
There was a problem hiding this comment.
aci.LoginParams could have a Validate function or something
There was a problem hiding this comment.
moved to a Validate() on LoginParams (and called from the corba command)
gtardif
force-pushed
the
azure_sp_login
branch
2 times, most recently
from
5113013
to
3846ab5
Compare
aci/backend_test.go
Outdated
| ClientID: "someID", | ||
| }) | ||
|
|
||
| assert.Error(t, err, "for Service Principal login, 3 options must be specified : --client-id, --client-secret and --tenant-id") |
There was a problem hiding this comment.
| assert.Error(t, err, "for Service Principal login, 3 options must be specified : --client-id, --client-secret and --tenant-id") | |
| assert.Error(t, err, "for Service Principal login, 3 options must be specified: --client-id, --client-secret and --tenant-id") |
gtardif
force-pushed
the
azure_sp_login
branch
from
3846ab5
to
ed72123
Compare
…n. Use it in E2E tests
gtardif
force-pushed
the
azure_sp_login
branch
from
ed72123
to
ec5f4e9
Compare
Co-authored-by: Djordje Lukic <djordje.lukic@docker.com>
What I did
Expose service principal login in the CLI, use it in E2E tests.
Related issue
docker-archive/cloud-integration-beta#21
(not mandatory) A picture of a cute animal, if possible in relation with what you did
