Repository: jenkins-x/lighthouse
Pull request #1307 (2021-05-12 04:38:45)

fix: fix uses resolver using incorrect version of uses:task.yaml
Test case in TestUsesInPRBranch, test data in pkg/triggerconfig/inrepo/test_data/myorg/uses-cache-issue

The fix was to include the SHA in the uses resolver cache key so that a different entry exists for master and the pr branch for a given path.

cameronbraid: Fixes https://github.com/jenkins-x/lighthouse/issues/1306
jenkins-x-bot-test: Hi @cameronbraid. Thanks for your PR.

I'm waiting for a [jenkins-x](https://github.com/orgs/jenkins-x/people) or [todo](https://github.com/orgs/todo/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/todo/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://jenkins-x.io/v3/develop/reference/chatops/?repo=jenkins-x%2Flighthouse).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository.
</details>

jenkins-x-bot-test: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **cagiti**
You can assign the PR to them by writing `/assign @cagiti` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://jenkins-x.io/v3/develop/reference/chatops/?repo=jenkins-x%2Flighthouse).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/jenkins-x/lighthouse/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["cagiti"]} -->

Patch:
diff --git a/go.sum b/go.sum
index a88e7876..3fe87f1d 100644
--- a/go.sum
+++ b/go.sum
@@ -435,7 +435,6 @@ github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdv
 github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
-github.com/hashicorp/go-version v1.2.1 h1:zEfKbn2+PDgroKdiOzqiE8rsmLqU2uwi5PB5pBJ3TkI=
 github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/hashicorp/go-version v1.3.0 h1:McDWVJIU/y+u1BRV06dPaLfLCaT7fUTJLp5r04x7iNw=
 github.com/hashicorp/go-version v1.3.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
@@ -465,8 +464,8 @@ github.com/influxdata/tdigest v0.0.0-20181121200506-bf2b5ad3c0a9/go.mod h1:Js0mq
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
 github.com/jcmturner/gofork v0.0.0-20190328161633-dc7c13fece03/go.mod h1:MK8+TM0La+2rjBD4jE12Kj1pCCxK7d2LK/UM3ncEo0o=
 github.com/jcmturner/gofork v1.0.0/go.mod h1:MK8+TM0La+2rjBD4jE12Kj1pCCxK7d2LK/UM3ncEo0o=
-github.com/jenkins-x/go-scm v1.7.2 h1:EWUHRFtJoLFInBMGkfceMqhrzIS8E+L2gKjJQCbnDgY=
-github.com/jenkins-x/go-scm v1.7.2/go.mod h1:z7xTO9/VzqW3xEbEMH2z5cpOGrZ8+nOHOWfU1ngFGxs=
+github.com/jenkins-x/go-scm v1.8.2 h1:GMEUz6ap0pAeyUkwWP3SQxJoz/qN+tJmCkr9sWvbXs4=
+github.com/jenkins-x/go-scm v1.8.2/go.mod h1:z7xTO9/VzqW3xEbEMH2z5cpOGrZ8+nOHOWfU1ngFGxs=
 github.com/jenkins-x/pipeline v0.3.2-0.20210223153617-0d1186b27496 h1:yN90dXuD7TkAkYwKr5h2fT7bjA7osK7tMhuz31qL7gE=
 github.com/jenkins-x/pipeline v0.3.2-0.20210223153617-0d1186b27496/go.mod h1:y1XwzBDu4EmOCfoeV2cUW2icCto/izTyjEKK5j0UmwA=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
diff --git a/pkg/triggerconfig/inrepo/resolver_cache.go b/pkg/triggerconfig/inrepo/resolver_cache.go
index 0b54cdff..fc6a4b68 100644
--- a/pkg/triggerconfig/inrepo/resolver_cache.go
+++ b/pkg/triggerconfig/inrepo/resolver_cache.go
@@ -6,6 +6,8 @@ import (
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 )
 
+const DELIMIER = "#"
+
 // ResolverCache a cache of data and pipelines to minimise
 // the git cloning with in repo configurations
 type ResolverCache struct {
@@ -23,45 +25,45 @@ func NewResolverCache() *ResolverCache {
 }
 
 // GetData gets data from the cache if available or returns nil
-func (c *ResolverCache) GetData(sourceURI string) []byte {
+func (c *ResolverCache) GetData(sourceURI, ref string) []byte {
 	if c == nil || sourceURI == "" {
 		return nil
 	}
 	var answer []byte
 	c.lock.Lock()
-	answer = c.dataCache[sourceURI]
+	answer = c.dataCache[sourceURI+DELIMIER+ref]
 	c.lock.Unlock()
 	return answer
 }
 
 // SetData updates the cache
-func (c *ResolverCache) SetData(sourceURI string, value []byte) {
+func (c *ResolverCache) SetData(sourceURI, ref string, value []byte) {
 	if c == nil || len(value) == 0 {
 		return
 	}
 	c.lock.Lock()
-	c.dataCache[sourceURI] = value
+	c.dataCache[sourceURI+DELIMIER+ref] = value
 	c.lock.Unlock()
 }
 
 // GetPipelineRun gets the PipelineRun from the cache if available or returns nil
-func (c *ResolverCache) GetPipelineRun(sourceURI string) *tektonv1beta1.PipelineRun {
+func (c *ResolverCache) GetPipelineRun(sourceURI, ref string) *tektonv1beta1.PipelineRun {
 	if c == nil || sourceURI == "" {
 		return nil
 	}
 	var answer *tektonv1beta1.PipelineRun
 	c.lock.Lock()
-	answer = c.pipelineCache[sourceURI]
+	answer = c.pipelineCache[sourceURI+DELIMIER+ref]
 	c.lock.Unlock()
 	return answer
 }
 
 // SetPipelineRun updates the cache
-func (c *ResolverCache) SetPipelineRun(sourceURI string, value *tektonv1beta1.PipelineRun) {
+func (c *ResolverCache) SetPipelineRun(sourceURI string, ref string, value *tektonv1beta1.PipelineRun) {
 	if c == nil || value == nil {
 		return
 	}
 	c.lock.Lock()
-	c.pipelineCache[sourceURI] = value
+	c.pipelineCache[sourceURI+DELIMIER+ref] = value
 	c.lock.Unlock()
 }
diff --git a/pkg/triggerconfig/inrepo/uses_resolver.go b/pkg/triggerconfig/inrepo/uses_resolver.go
index 11ed0294..98a13e20 100644
--- a/pkg/triggerconfig/inrepo/uses_resolver.go
+++ b/pkg/triggerconfig/inrepo/uses_resolver.go
@@ -41,7 +41,7 @@ var (
 // UsesSteps lets resolve the sourceURI to a PipelineRun and find the step or steps
 // for the given task name and/or step name then lets apply any overrides from the step
 func (r *UsesResolver) UsesSteps(sourceURI string, taskName string, step tektonv1beta1.Step, ts *tektonv1beta1.TaskSpec, loc *UseLocation) ([]tektonv1beta1.Step, error) {
-	pr := r.Cache.GetPipelineRun(sourceURI)
+	pr := r.Cache.GetPipelineRun(sourceURI, r.SHA)
 	if pr == nil || ignoreUsesCache {
 		data, err := r.GetData(sourceURI, false)
 		if err != nil {
@@ -58,7 +58,7 @@ func (r *UsesResolver) UsesSteps(sourceURI string, taskName string, step tektonv
 		if pr == nil {
 			return nil, errors.Errorf("no PipelineRun for URI %s", sourceURI)
 		}
-		r.Cache.SetPipelineRun(sourceURI, pr)
+		r.Cache.SetPipelineRun(sourceURI, r.SHA, pr)
 	}
 
 	useTS, err := r.findSteps(sourceURI, pr.DeepCopy(), taskName, step)
@@ -88,7 +88,7 @@ func (r *UsesResolver) UsesSteps(sourceURI string, taskName string, step tektonv
 
 // GetData gets the data from the given source URI
 func (r *UsesResolver) GetData(path string, ignoreNotExist bool) ([]byte, error) {
-	data := r.Cache.GetData(path)
+	data := r.Cache.GetData(path, r.SHA)
 	if len(data) > 0 {
 		return data, nil
 	}
@@ -151,7 +151,7 @@ func (r *UsesResolver) GetData(path string, ignoreNotExist bool) ([]byte, error)
 		return nil, errors.Wrapf(err, "failed to find file %s in repo %s/%s with sha %s", path, owner, repo, sha)
 	}
 	if gitURI != nil {
-		r.Cache.SetData(path, data)
+		r.Cache.SetData(path, r.SHA, data)
 	}
 	return data, nil
 }


Test patch:
diff --git a/pkg/triggerconfig/inrepo/calculate_test.go b/pkg/triggerconfig/inrepo/calculate_test.go
index f9ec9fb5..02f01b7e 100644
--- a/pkg/triggerconfig/inrepo/calculate_test.go
+++ b/pkg/triggerconfig/inrepo/calculate_test.go
@@ -145,3 +145,49 @@ func TestTriggersInBranchMergeToMaster(t *testing.T) {
 	assert.Len(t, cfg.Postsubmits[fullName], 1, "postsubmits for repo %s", fullName)
 
 }
+
+func TestIssue1306(t *testing.T) {
+
+	/**
+	*
+	* The issue is that the master version of shared-task.yaml is used even though the PR branch provides an update
+	*
+	 */
+	owner := "myorg"
+	repo := "issue-1306"
+
+	fullName := scm.Join(owner, repo)
+
+	scmClient, fakeData := fakescm.NewDefault()
+	scmProvider := scmprovider.ToClient(scmClient, "my-bot")
+
+	fakeData.Repositories = []*scm.Repository{
+		{
+			Namespace: owner,
+			Name:      repo,
+			FullName:  fullName,
+			Branch:    "master",
+		},
+	}
+
+	enabled := true
+	sharedConfig := &config.Config{
+		ProwConfig: config.ProwConfig{
+			InRepoConfig: lighthouse.InRepoConfig{
+				Enabled: map[string]*bool{
+					fullName: &enabled,
+				},
+			},
+		},
+	}
+	sharedPluginConfig := &plugins.Configuration{}
+
+	fileBrowsers, err := filebrowser.NewFileBrowsers(filebrowser.GitHubURL, filebrowser.NewFileBrowserFromScmClient(scmProvider))
+	require.NoError(t, err, "failed to create filebrowsers")
+
+	cfg, _, err := inrepo.Generate(fileBrowsers, inrepo.NewResolverCache(), sharedConfig, sharedPluginConfig, owner, repo, "pr1")
+	require.NoError(t, err, "failed to calculate in repo config")
+
+	assert.Contains(t, cfg.Presubmits[fullName][0].Base.PipelineRunSpec.PipelineSpec.Tasks[0].TaskSpec.Steps[0].Script, "ubuntu-pr1")
+
+}
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/pullrequest.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/pullrequest.yaml
new file mode 100644
index 00000000..f11fd44f
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/pullrequest.yaml
@@ -0,0 +1,12 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  creationTimestamp: null
+  name: release
+spec:
+  pipelineSpec:
+    tasks:
+    - name: release
+      taskSpec:
+        steps:
+        - image: uses:.lighthouse/jenkins-x/shared-task.yaml
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/shared-task.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/shared-task.yaml
new file mode 100644
index 00000000..9348c6f2
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/shared-task.yaml
@@ -0,0 +1,11 @@
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: task1
+spec:
+  steps:
+  - name: step-1
+    image: ubuntu
+    script: |
+      #!/usr/bin/env bash
+      echo ubuntu-master
\ No newline at end of file
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/triggers.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/triggers.yaml
new file mode 100644
index 00000000..1416f2a6
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/master/.lighthouse/jenkins-x/triggers.yaml
@@ -0,0 +1,9 @@
+apiVersion: config.lighthouse.jenkins-x.io/v1alpha1
+kind: TriggerConfig
+spec:
+  presubmits:
+  - name: pullrequest
+    context: "pr"
+    always_run: true
+    optional: false
+    source: "pullrequest.yaml"
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/pullrequest.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/pullrequest.yaml
new file mode 100644
index 00000000..f11fd44f
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/pullrequest.yaml
@@ -0,0 +1,12 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  creationTimestamp: null
+  name: release
+spec:
+  pipelineSpec:
+    tasks:
+    - name: release
+      taskSpec:
+        steps:
+        - image: uses:.lighthouse/jenkins-x/shared-task.yaml
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/shared-task.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/shared-task.yaml
new file mode 100644
index 00000000..325e16ea
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/shared-task.yaml
@@ -0,0 +1,11 @@
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: shared-task
+spec:
+  steps:
+  - name: step-1
+    image: ubuntu
+    script: |
+      #!/usr/bin/env bash
+      echo ubuntu-pr1
\ No newline at end of file
diff --git a/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/triggers.yaml b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/triggers.yaml
new file mode 100644
index 00000000..1416f2a6
--- /dev/null
+++ b/pkg/triggerconfig/inrepo/test_data/myorg/issue-1306/refs/pr1/.lighthouse/jenkins-x/triggers.yaml
@@ -0,0 +1,9 @@
+apiVersion: config.lighthouse.jenkins-x.io/v1alpha1
+kind: TriggerConfig
+spec:
+  presubmits:
+  - name: pullrequest
+    context: "pr"
+    always_run: true
+    optional: false
+    source: "pullrequest.yaml"