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

Using GitArtifact as a k8s trigger source fails when using ref #810

Closed
PanagiotisS opened this issue Jul 12, 2023 · 2 comments
Closed

Using GitArtifact as a k8s trigger source fails when using ref #810

PanagiotisS opened this issue Jul 12, 2023 · 2 comments

Comments

@PanagiotisS
Copy link

Describe the bug
Using GitArtifact as a k8s trigger source fails when using ref

To Reproduce
Steps to reproduce the behavior:

  1. Example sensor
---
apiVersion: argoproj.io/v1alpha1
kind: Sensor
metadata:
  name: webhook-sensor
spec:
  revisionHistoryLimit: 3
  template:
    container:
      volumeMounts:
        - mountPath: /git/argoproj
          name: argoproj
      env:
        - name: DEBUG_LOG
          value: "true"
    volumes:
      - name: argoproj
        emptyDir: {}
    serviceAccountName: operate-workflow-sa
  dependencies:
    - name: gitea-dep
      eventSourceName: webhook-source
      eventName: gitea
  triggers:
    - template:
        k8s:
          operation: create
          source:
            git:
              cloneDirectory: /git/argoproj
              creds:
                password:
                  key: password
                  name: gitea-secret
                username:
                  key: username
                  name: gitea-secret
              filePath: .argo/workflow.yaml
              ref: refs/heads/feature/argo-workflow
              url: http://gitea.internal/test/test.git 
        name: workflow-trigger-with-git-source
  1. Error message
webhook-sensor-sensor-9qlwv-78b95697cf-dpcqb main 2023-07-04T14:01:49.805703295+03:00 2023-07-04T11:01:49.805Z  ERROR   argo-events.sensor      sensors/listene
r.go:362        Failed to execute a trigger     {"sensorName": "webhook-sensor", "error": "failed to fetch artifact, failed after retries: failed to fetch. err
: empty git-upload-pack given", "triggerName": "workflow-trigger-with-git-source", "triggeredBy": ["gitea-dep"], "triggeredByEvents": ["35393837613432372d34373
6652d343564622d623163342d393964366438353535383038"]} 

Expected behavior
Expected to work. For example if instead of ref: refs/heads/feature/argo-workflow
I use branch: feature/argo-workflow it works. It clones the git repo, and submits the workflow without issues.

Environment (please complete the following information):

  • Kubernetes: 1.26.1
  • Argo: v3.5.8
  • Argo Events: v1.8.0

Additional context
Related issues:

go-git: empty git-upload-pack given errors since v5.4.0 #328

related issue:
go-git: Pull: error: empty git-upload-pack given

fluxcd solution: upgrade to unreleased v5.8.0
5242551eae/go.mod (L7)

metal-robot solution: Downgrade to v5.3.0
metal-stack/metal-robot#55

Attempted Solution

Upgrading to the same un-rleased v5.8.0 as fluxcd, fixes the issue with the fetch command.

However, afterwards it tries to pull but it fails with error

webhook-sensor-sensor-bcm4w-6898d9c855-nnrzg main 2023-07-11T08:23:24.315Z      ERROR   argo-events.sensor      sensors/listener.go:362 Failed to execute a trigger     {"sensorName": "webhook-sensor", "error": "failed to fetch artifact, failed after retries: failed to pull latest updates. err: empty git-upload-pack given", "triggerName": "workflow-trigger-with-git-source", "triggeredBy": ["gitea-dep"], "triggeredByEvents": ["35623862613065362d356431372d343633322d613466372d666262396239316663343734"]}

https://github.com/argoproj/argo-events/blob/46e2d01f397515778f83779430f9012b00a03c0b/sensors/artifacts/git.go#L180-L194

The comment, correctly notes that is not necessary to pull.

Removing this block, we can successfully clone a repo when using ref: refs/heads/feature/argo-workflow or when using branch: feature/argo-workflow.

Steps to successfully fix:

  1. git checkout v1.8.0
  2. Apply the following patch
diff --git a/go.mod b/go.mod
index 6d039e7f..d250c88f 100644
--- a/go.mod
+++ b/go.mod
@@ -4,10 +4,12 @@ go 1.20
 
 retract v1.15.1 // Contains retractions only.
 
 retract v1.15.0 // Published accidentally.
 
+replace github.com/go-git/go-git/v5 => github.com/go-git/go-git/v5 v5.7.1-0.20230702134234-dd4e2b7f4b01
+
 require (
 	cloud.google.com/go/compute/metadata v0.2.3
 	cloud.google.com/go/pubsub v1.30.1
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20221103172237-443f56ff4ba8
 	github.com/Azure/azure-event-hubs-go/v3 v3.5.0
diff --git a/sensors/artifacts/git.go b/sensors/artifacts/git.go
index edabe560..ff9da0ea 100644
--- a/sensors/artifacts/git.go
+++ b/sensors/artifacts/git.go
@@ -175,25 +175,11 @@ func (g *GitArtifactReader) readFromRepository(r *git.Repository, dir string) ([
 
 	if err := w.Checkout(g.getBranchOrTag()); err != nil {
 		return nil, fmt.Errorf("failed to checkout. err: %+v", err)
 	}
 
-	// In the case of a specific given ref, it shouldn't be necessary to pull
-	if g.artifact.Ref != "" {
-		pullOpts := &git.PullOptions{
-			RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
-			ReferenceName:     g.getBranchOrTag().Branch,
-			Force:             true,
-		}
-		if auth != nil {
-			pullOpts.Auth = auth
-		}
 
-		if err := w.Pull(pullOpts); err != nil && err != git.NoErrAlreadyUpToDate {
-			return nil, fmt.Errorf("failed to pull latest updates. err: %+v", err)
-		}
-	}
 	filePath := fmt.Sprintf("%s/%s", dir, g.artifact.FilePath)
 	// symbol link is not allowed due to security concern
 	isSymbolLink, err := isSymbolLink(filePath)
 	if err != nil {
 		return nil, err
  1. go mod tidy
    (this step updates a bunch of other go dependencies as well

Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

@pjbgf
Copy link
Member

pjbgf commented Jul 12, 2023

@PanagiotisS can you share go-git-focused steps to reproduce please?

@PanagiotisS
Copy link
Author

PanagiotisS commented Jul 12, 2023

Hi. Sorry. My bad. I posted this in the wrong repository. Really sorry.

Edit: This is already fixed in this repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants