builder: skip name validation for docker context

[crazy-max]

related to docker/for-win#13543

Although a builder from the store cannot be created unless it has a valid name, this is not the case for a Docker context.

We should skip name validation when checking a node from the store and fall back to finding one from Docker context instead.

[thaJeztah]

Interesting; it looks like context-store in docker/cli does have a validation function;

buildx/vendor/github.com/docker/cli/cli/context/store/store.go

Lines 214 to 226 in bd672ea

	// ValidateContextName checks a context name is valid.
	func ValidateContextName(name string) error {
	if name == "" {
	return errors.New("context name cannot be empty")
	}
	if name == "default" {
	return errors.New(`"default" is a reserved context name`)
	}
	if !restrictedNameRegEx.MatchString(name) {
	return errors.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern)
	}
	return nil
	}

buildx/vendor/github.com/docker/cli/cli/context/store/store.go

Lines 22 to 24 in bd672ea

	const restrictedNamePattern = "^[a-zA-Z0-9][a-zA-Z0-9_.+-]+$"
	var restrictedNameRegEx = regexp.MustCompile(restrictedNamePattern)

But I was actually looking at other parts of the code, and it looks like validation is done externally 😞 https://github.com/docker/cli/blob/085d5c281612298583d19806aa8ec637b572e4cc/cli/command/context/create.go#L124-L135

func checkContextNameForCreation(s store.Reader, name string) error {
	if err := store.ValidateContextName(name); err != nil {
		return err
	}
	if _, err := s.GetMetadata(name); !errdefs.IsNotFound(err) {
		if err != nil {
			return errors.Wrap(err, "error while getting existing contexts")
		}
		return errors.Errorf("context %q already exists", name)
	}
	return nil
}

https://github.com/docker/cli/blob/085d5c281612298583d19806aa8ec637b572e4cc/cli/command/context/create.go#L71-L76

// RunCreate creates a Docker context
func RunCreate(cli command.Cli, o *CreateOptions) error {
	s := cli.ContextStore()
	err := checkContextNameForCreation(s, o.Name)
	if err != nil {
		return err
	}
	swit

So I wonder if there's a bug at hand here, where validation was missed?

[crazy-max]

Oh good catch @thaJeztah. Yeah there's smth fishy here 🤔

[thaJeztah]

Yeah, was looking "why isn't the validation done by the context store code?", and then saw this PR fly by in my notifications 😂

Is the case here that BuildKit / buildx can use either hostname or context-name, and doesn't know which is the case?