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

Fix: Update GetNameAndNamespace Parameters #7984

Merged
merged 1 commit into from
May 27, 2024
Merged
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
6 changes: 5 additions & 1 deletion pkg/remote/resolution/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/scheme"
"github.com/tektoncd/pipeline/pkg/remote"
resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
Expand Down Expand Up @@ -73,7 +74,10 @@ func (resolver *Resolver) List(_ context.Context) ([]remote.ResolvedObject, erro
}

func buildRequest(resolverName string, owner kmeta.OwnerRefable, name string, namespace string, params v1.Params) (*resolutionRequest, error) {
name, namespace, err := remoteresource.GetNameAndNamespace(resolverName, owner, name, namespace, params)
rr := &v1beta1.ResolutionRequestSpec{
Params: params,
}
name, namespace, err := remoteresource.GetNameAndNamespace(resolverName, owner, name, namespace, rr)
if err != nil {
return nil, err
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/remoteresolution/remote/resolution/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"

v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
"github.com/tektoncd/pipeline/pkg/remote"
resolution "github.com/tektoncd/pipeline/pkg/remote/resolution"
remoteresource "github.com/tektoncd/pipeline/pkg/remoteresolution/resource"
Expand Down Expand Up @@ -69,15 +70,21 @@ func (resolver *Resolver) List(_ context.Context) ([]remote.ResolvedObject, erro
func buildRequest(resolverName string, owner kmeta.OwnerRefable, resolverPayload *remoteresource.ResolverPayload) (*resolutionRequest, error) {
var name string
var namespace string
var url string
var params v1.Params
if resolverPayload != nil {
name = resolverPayload.Name
namespace = resolverPayload.Namespace
if resolverPayload.ResolutionSpec != nil {
params = resolverPayload.ResolutionSpec.Params
url = resolverPayload.ResolutionSpec.URL
}
}
name, namespace, err := resource.GetNameAndNamespace(resolverName, owner, name, namespace, params)
rr := &v1beta1.ResolutionRequestSpec{
Params: params,
Aleromerog marked this conversation as resolved.
Show resolved Hide resolved
URL: url,
}
name, namespace, err := resource.GetNameAndNamespace(resolverName, owner, name, namespace, rr)
if err != nil {
return nil, err
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/remoteresolution/remote/resolution/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
resv1beta1 "github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"

"github.com/tektoncd/pipeline/pkg/remote"
"github.com/tektoncd/pipeline/pkg/remote/resolution"
remoteresource "github.com/tektoncd/pipeline/pkg/remoteresolution/resource"
Expand Down Expand Up @@ -142,12 +144,18 @@ func TestBuildRequestV2(t *testing.T) {
name string
targetName string
targetNamespace string
url string
}{{
name: "just owner",
}, {
name: "with target name and namespace",
targetName: "some-object",
targetNamespace: "some-ns",
}, {
name: "with target name, namespace, and url",
targetName: "some-object",
targetNamespace: "some-ns",
url: "scheme://value",
}} {
t.Run(tc.name, func(t *testing.T) {
owner := &v1beta1.PipelineRun{
Expand All @@ -158,6 +166,7 @@ func TestBuildRequestV2(t *testing.T) {
}

rr := &remoteresource.ResolverPayload{Name: tc.targetName, Namespace: tc.targetNamespace}
rr.ResolutionSpec = &resv1beta1.ResolutionRequestSpec{URL: tc.url}
req, err := buildRequest("git", owner, rr)
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand Down
7 changes: 5 additions & 2 deletions pkg/resolution/resource/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ func GenerateDeterministicName(prefix, base string, params v1.Params) (string, e
return GenerateDeterministicNameFromSpec(prefix, base, &v1beta1.ResolutionRequestSpec{Params: params})
}

func GetNameAndNamespace(resolverName string, owner kmeta.OwnerRefable, name string, namespace string, params v1.Params) (string, string, error) {
// GetNameAndNamespace determines the name and namespace for a resource request.
// It prioritizes explicit values, falling back to the owning object and "default" namespace.
// If needed, it generates a deterministic name to prevent duplicate requests within a context.
func GetNameAndNamespace(resolverName string, owner kmeta.OwnerRefable, name string, namespace string, req *v1beta1.ResolutionRequestSpec) (string, string, error) {
Aleromerog marked this conversation as resolved.
Show resolved Hide resolved
if name == "" {
name = owner.GetObjectMeta().GetName()
namespace = owner.GetObjectMeta().GetNamespace()
Expand All @@ -62,7 +65,7 @@ func GetNameAndNamespace(resolverName string, owner kmeta.OwnerRefable, name str
// prevents multiple requests being issued for the same
// pipelinerun's pipelineRef or taskrun's taskRef.
remoteResourceBaseName := namespace + "/" + name
name, err := GenerateDeterministicNameFromSpec(resolverName, remoteResourceBaseName, &v1beta1.ResolutionRequestSpec{Params: params})
name, err := GenerateDeterministicNameFromSpec(resolverName, remoteResourceBaseName, req)
if err != nil {
return "", "", fmt.Errorf("error generating name for taskrun %s/%s: %w", namespace, name, err)
}
Expand Down