-
Notifications
You must be signed in to change notification settings - Fork 1.9k
hub resolver: remove placeholder URL workaround in Validate() #9684
Description
Context
PR #9465 added multi-URL support and a per-resolution url param to the Hub Resolver. To bypass the ValidateParams() check that requires TEKTON_HUB_API to be set for Tekton Hub type, the code passes placeholder strings ("url-param-configured", "configmap-urls-configured") as the tektonHubURL argument:
// pkg/remoteresolution/resolver/hub/resolver.go
if hasURLOverride {
return hub.ValidateParams(ctx, req.Params, "url-param-configured")
}
configURLs, _ := parseURLList(conf[ConfigTektonHubURLs])
if len(configURLs) > 0 {
return hub.ValidateParams(ctx, req.Params, "configmap-urls-configured")
}This works because validateParams() only checks tektonHubURL == "" — any non-empty string passes. But it's a code smell: the placeholder strings look like real URLs to readers, and the workaround is brittle if validation ever becomes stricter.
Proposed fix
Refactor validateParams() in pkg/resolution/resolver/hub/resolver.go to accept a richer signal (e.g. a bool hasHubURL or an options struct) instead of relying on a non-empty string. This would let the remoteresolution Validate() method cleanly express "a URL is available via param or ConfigMap" without faking a URL value.
Alternatively, since the deprecated pkg/resolution/resolver/hub package will eventually be removed, the validation logic in pkg/remoteresolution/resolver/hub/resolver.go could be made self-contained (it already has its own validateParamsForResolve()) and stop calling through to the shared hub.ValidateParams() entirely.
Origin
Identified during review of #9465 — see inline comment.
/kind cleanup
Metadata
Metadata
Assignees
Labels
Type
Projects
Status