cli/command: explicitly map AuthConfig fields instead of a direct cast
Commit [cli@27b2797] forked the AuthConfig type from the API, and changed
existing code to do a direct cast / convert of the forked type to the API
type. This can cause issues if the API types diverges, such as the removal
of the Email field.
This patch explicitly maps each field to the corresponding API type, but
adds some TODOs, because various code-paths only included a subset of the
fields, which may be intentional for fields that were meant to be handled
on the daemon / registry-client only.
We should evaluate these conversions to make sure these fields should
be sent from the client or not (and possibly even removed from the API
type).
[cli@27b2797]: 27b2797f7d
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
@ -317,8 +317,17 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
|
||||
configFile := dockerCli.ConfigFile()
|
||||
creds, _ := configFile.GetAllCredentials()
|
||||
authConfigs := make(map[string]registrytypes.AuthConfig, len(creds))
|
||||
for k, auth := range creds {
|
||||
authConfigs[k] = registrytypes.AuthConfig(auth)
|
||||
for k, authConfig := range creds {
|
||||
authConfigs[k] = registrytypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
}
|
||||
}
|
||||
buildOpts := imageBuildOptions(dockerCli, options)
|
||||
buildOpts.Version = buildtypes.BuilderV1
|
||||
|
||||
@ -50,7 +50,16 @@ func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInf
|
||||
}
|
||||
|
||||
a, _ := cfg.GetAuthConfig(configKey)
|
||||
return registrytypes.AuthConfig(a)
|
||||
return registrytypes.AuthConfig{
|
||||
Username: a.Username,
|
||||
Password: a.Password,
|
||||
ServerAddress: a.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: a.Auth,
|
||||
IdentityToken: a.IdentityToken,
|
||||
RegistryToken: a.RegistryToken,
|
||||
}
|
||||
}
|
||||
|
||||
// GetDefaultAuthConfig gets the default auth config given a serverAddress
|
||||
@ -69,9 +78,17 @@ func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serve
|
||||
}, err
|
||||
}
|
||||
}
|
||||
authCfg.ServerAddress = serverAddress
|
||||
authCfg.IdentityToken = ""
|
||||
return registrytypes.AuthConfig(authCfg), nil
|
||||
|
||||
return registrytypes.AuthConfig{
|
||||
Username: authCfg.Username,
|
||||
Password: authCfg.Password,
|
||||
ServerAddress: serverAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authCfg.Auth,
|
||||
IdentityToken: "",
|
||||
RegistryToken: authCfg.RegistryToken,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// PromptUserForCredentials handles the CLI prompt for the user to input
|
||||
@ -186,7 +203,16 @@ func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (strin
|
||||
return "", err
|
||||
}
|
||||
|
||||
encodedAuth, err := authconfig.Encode(registrytypes.AuthConfig(authConfig))
|
||||
encodedAuth, err := authconfig.Encode(registrytypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
@ -238,12 +238,30 @@ func loginWithDeviceCodeFlow(ctx context.Context, dockerCLI command.Cli) (msg st
|
||||
return "", err
|
||||
}
|
||||
|
||||
response, err := loginWithRegistry(ctx, dockerCLI.Client(), registrytypes.AuthConfig(*authConfig))
|
||||
response, err := loginWithRegistry(ctx, dockerCLI.Client(), registrytypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
if err = storeCredentials(dockerCLI.ConfigFile(), registrytypes.AuthConfig(*authConfig)); err != nil {
|
||||
if err = storeCredentials(dockerCLI.ConfigFile(), registrytypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
}); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
@ -252,7 +270,16 @@ func loginWithDeviceCodeFlow(ctx context.Context, dockerCLI command.Cli) (msg st
|
||||
|
||||
func storeCredentials(cfg *configfile.ConfigFile, authConfig registrytypes.AuthConfig) error {
|
||||
creds := cfg.GetCredentialsStore(authConfig.ServerAddress)
|
||||
if err := creds.Store(configtypes.AuthConfig(authConfig)); err != nil {
|
||||
if err := creds.Store(configtypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
}); err != nil {
|
||||
return fmt.Errorf("error saving credentials: %v", err)
|
||||
}
|
||||
|
||||
|
||||
@ -103,7 +103,16 @@ func getAuth(dockerCLI command.Cli, reposName string) (encodedAuth string, err e
|
||||
// "no credentials found"). We'll get an error when search failed,
|
||||
// so fine to ignore in most situations.
|
||||
authConfig, _ := dockerCLI.ConfigFile().GetAuthConfig(authCfgKey)
|
||||
return authconfig.Encode(registrytypes.AuthConfig(authConfig))
|
||||
return authconfig.Encode(registrytypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
})
|
||||
}
|
||||
|
||||
// splitReposSearchTerm breaks a search term into an index name and remote name
|
||||
|
||||
@ -59,8 +59,17 @@ func TestGetDefaultAuthConfig(t *testing.T) {
|
||||
},
|
||||
}
|
||||
cfg := configfile.New("filename")
|
||||
for _, authCfg := range testAuthConfigs {
|
||||
assert.Check(t, cfg.GetCredentialsStore(authCfg.ServerAddress).Store(configtypes.AuthConfig(authCfg)))
|
||||
for _, authConfig := range testAuthConfigs {
|
||||
assert.Check(t, cfg.GetCredentialsStore(authConfig.ServerAddress).Store(configtypes.AuthConfig{
|
||||
Username: authConfig.Username,
|
||||
Password: authConfig.Password,
|
||||
ServerAddress: authConfig.ServerAddress,
|
||||
|
||||
// TODO(thaJeztah): Are these expected to be included?
|
||||
Auth: authConfig.Auth,
|
||||
IdentityToken: authConfig.IdentityToken,
|
||||
RegistryToken: authConfig.RegistryToken,
|
||||
}))
|
||||
}
|
||||
for _, tc := range testCases {
|
||||
serverAddress := tc.inputServerAddress
|
||||
|
||||
Reference in New Issue
Block a user