Skip to content
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

cli/command: remove unused args from ResolveDefaultContext() (step 2) #3510

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
cli.contextStore = &ContextStoreWithDefault{
Store: store.New(config.ContextStoreDir(), cli.contextStoreConfig),
Resolver: func() (*DefaultContext, error) {
return ResolveDefaultContext(cli.options, cli.contextStoreConfig)
return ResolveDefaultContext(cli.options)
},
}
return nil
Expand All @@ -244,12 +244,10 @@ func NewAPIClientFromFlags(opts *cliflags.ClientOptions, configFile *configfile.
if opts.Context != "" && len(opts.Hosts) > 0 {
return nil, errors.New("conflicting options: either specify --host or --context, not both")
}

storeConfig := DefaultContextStoreConfig()
contextStore := &ContextStoreWithDefault{
Store: store.New(config.ContextStoreDir(), storeConfig),
Store: store.New(config.ContextStoreDir(), DefaultContextStoreConfig()),
Resolver: func() (*DefaultContext, error) {
return ResolveDefaultContext(opts, storeConfig)
return ResolveDefaultContext(opts)
},
}
endpoint, err := resolveDockerEndpoint(contextStore, resolveContextName(opts, configFile))
Expand Down
67 changes: 16 additions & 51 deletions cli/command/defaultcontextstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,66 +34,31 @@ type ContextStoreWithDefault struct {
Resolver DefaultContextResolver
}

// EndpointDefaultResolver is implemented by any EndpointMeta object
// which wants to be able to populate the store with whatever their default is.
type EndpointDefaultResolver interface {
Copy link
Member Author

@thaJeztah thaJeztah Apr 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is used in buildx (see docker/buildx#1054); let me check how/why and if it's strictly needed

+ xx-go build -ldflags '-X github.com/docker/buildx/version.Version=9e61f69 -X github.com/docker/buildx/version.Revision=9e61f69a5b83c3d6ee5545e488d5ad430c894276 -X github.com/docker/buildx/version.Package=github.com/docker/buildx -w -s' -o /usr/bin/buildx ./cmd/buildx
# github.com/docker/buildx/driver/kubernetes/context
driver/kubernetes/context/load.go:31:7: undefined: command.EndpointDefaultResolver

// ResolveDefault returns values suitable for storing in store.Metadata.Endpoints
// and store.ContextTLSData.Endpoints.
//
// An error is only returned for something fatal, not simply
// the lack of a default (e.g. because the config file which
// would contain it is missing). If there is no default then
// returns nil, nil, nil.
ResolveDefault() (interface{}, *store.EndpointTLSData, error)
}

// ResolveDefaultContext creates a Metadata for the current CLI invocation parameters
func ResolveDefaultContext(opts *cliflags.ClientOptions, config store.Config) (*DefaultContext, error) {
contextTLSData := store.ContextTLSData{
Endpoints: make(map[string]store.EndpointTLSData),
}
contextMetadata := store.Metadata{
Endpoints: make(map[string]interface{}),
Metadata: DockerContext{
Description: "",
},
Name: DefaultContextName,
}

func ResolveDefaultContext(opts *cliflags.ClientOptions) (*DefaultContext, error) {
dockerEP, err := resolveDefaultDockerEndpoint(opts)
if err != nil {
return nil, err
}
contextMetadata.Endpoints[docker.DockerEndpoint] = dockerEP.EndpointMeta
contextTLSData := store.ContextTLSData{}
if dockerEP.TLSData != nil {
contextTLSData.Endpoints[docker.DockerEndpoint] = *dockerEP.TLSData.ToStoreTLSData()
}

if err := config.ForeachEndpointType(func(n string, get store.TypeGetter) error {
if n == docker.DockerEndpoint { // handled above
return nil
contextTLSData.Endpoints = map[string]store.EndpointTLSData{
docker.DockerEndpoint: *dockerEP.TLSData.ToStoreTLSData(),
}
ep := get()
if i, ok := ep.(EndpointDefaultResolver); ok {
meta, tls, err := i.ResolveDefault()
if err != nil {
return err
}
if meta == nil {
return nil
}
contextMetadata.Endpoints[n] = meta
if tls != nil {
contextTLSData.Endpoints[n] = *tls
}
}
// Nothing to be done
return nil
}); err != nil {
return nil, err
}

return &DefaultContext{Meta: contextMetadata, TLS: contextTLSData}, nil
return &DefaultContext{
Meta: store.Metadata{
Endpoints: map[string]interface{}{
docker.DockerEndpoint: dockerEP.EndpointMeta,
},
Metadata: DockerContext{
Description: "",
},
Name: DefaultContextName,
},
TLS: contextTLSData,
}, nil
}

// List implements store.Store's List
Expand Down
2 changes: 1 addition & 1 deletion cli/command/defaultcontextstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestDefaultContextInitializer(t *testing.T) {
TLSOptions: &tlsconfig.Options{
CAFile: "./testdata/ca.pem",
},
}, DefaultContextStoreConfig())
})
assert.NilError(t, err)
assert.Equal(t, "default", ctx.Meta.Name)
assert.DeepEqual(t, "ssh://someswarmserver", ctx.Meta.Endpoints[docker.DockerEndpoint].(docker.EndpointMeta).Host)
Expand Down
10 changes: 0 additions & 10 deletions cli/context/store/storeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@ func (c Config) SetEndpoint(name string, getter TypeGetter) {
c.endpointTypes[name] = getter
}

// ForeachEndpointType calls cb on every endpoint type registered with the Config
func (c Config) ForeachEndpointType(cb func(string, TypeGetter) error) error {
for n, ep := range c.endpointTypes {
if err := cb(n, ep); err != nil {
return err
}
}
return nil
}

// NewConfig creates a config object
func NewConfig(contextType TypeGetter, endpoints ...NamedTypeGetter) Config {
res := Config{
Expand Down
Loading