Skip to content

Commit

Permalink
fix: nil pointer ref for apps w/ multi sources
Browse files Browse the repository at this point in the history 
filterApplications() wasn't properly handling cases when an Argo
application had multiple sources defined.
  • Loading branch information
@vrivellino
vrivellino committed Jan 26, 2024
1 parent 2c0fd00 commit 25e9797
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 10 deletions.
127 changes: 125 additions & 2 deletions internal/argocd/argocd_testdata/payload-GET-applications-brief.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@
"summary": {},
"controllerNamespace": "argocd"
}
},
{
}, {
"metadata": {
"name": "argo-diff",
"namespace": "argocd",
Expand Down Expand Up @@ -284,6 +283,130 @@
},
"controllerNamespace": "argocd"
}
}, {
"metadata": {
"name": "argo-diff-multi",
"namespace": "argocd2",
"uid": "8aa5134a3-c334-46cd-a904-71e86bf8dd12",
"resourceVersion": "479656712",
"generation": 1,
"creationTimestamp": "2023-11-10T17:47:14Z",
"labels": {
"argocd.argoproj.io/instance": "argo-apps"
},
"annotations": {
"kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"argoproj.io/v1alpha1\",\"kind\":\"Application\",\"metadata\":{\"annotations\":{},\"labels\":{\"argocd.argoproj.io/instance\":\"argo-apps\"},\"name\":\"argo-diff-multi\",\"namespace\":\"argocd2\"},\"spec\":{\"destination\":{\"namespace\":\"argocd2\",\"server\":\"https://kubernetes.default.svc\"},\"project\":\"argocd-extras\",\"source\":{\"path\":\"k8s\",\"repoURL\":\"ssh://[email protected]/vince-riv/argo-diff.git\",\"targetRevision\":\"HEAD\"},\"syncPolicy\":{}}}\n"
},
"managedFields": [
{
"manager": "argocd-controller",
"operation": "Update",
"apiVersion": "argoproj.io/v1alpha1",
"time": "2023-11-10T17:49:21Z",
"fieldsType": "FieldsV1",
"fieldsV1": {}
},
{
"manager": "argocd-application-controller",
"operation": "Update",
"apiVersion": "argoproj.io/v1alpha1",
"time": "2023-11-10T17:49:22Z",
"fieldsType": "FieldsV1",
"fieldsV1": {}
}
]
},
"spec": {
"sources": [{
"repoURL": "https:://repo-hostname.dummy/charts",
"targetRevision": "1.2.3",
"chart": "argo-diff",
"helm": {
"passCredentials": true,
"valueFiles": ["$values/k8s/conf/values.yaml"]
}
},{
"repoURL": "ssh://[email protected]/vince-riv/argo-diff.git",
"targetRevision": "HEAD",
"ref": "values"
}],
"destination": {
"server": "https://kubernetes.default.svc",
"namespace": "argocd2"
},
"project": "argocd-extras",
"syncPolicy": {}
},
"status": {
"resources": [
{
"version": "v1",
"kind": "Service",
"namespace": "argocd2",
"name": "argo-diff-multi",
"status": "OutOfSync",
"health": {
"status": "Healthy"
}
},
{
"group": "apps",
"version": "v1",
"kind": "Deployment",
"namespace": "argocd2",
"name": "argo-diff-multi",
"status": "OutOfSync",
"health": {
"status": "Healthy"
}
},
{
"group": "bitnami.com",
"version": "v1alpha1",
"kind": "SealedSecret",
"namespace": "argocd2",
"name": "argo-diff-multi-env",
"status": "OutOfSync",
"health": {
"status": "Healthy"
}
},
{
"group": "traefik.containo.us",
"version": "v1alpha1",
"kind": "IngressRoute",
"namespace": "argocd2",
"name": "argocd-diff-multi",
"status": "OutOfSync"
}
],
"sync": {
"status": "OutOfSync",
"comparedTo": {
"source": {
"repoURL": "ssh://[email protected]/vince-riv/argo-diff.git",
"path": "k8s",
"targetRevision": "HEAD"
},
"destination": {
"server": "https://kubernetes.default.svc",
"namespace": "argocd"
}
},
"revision": "0341d95b8c70dc5f555fc8fe337e14b5496ff092"
},
"health": {
"status": "Healthy"
},
"reconciledAt": "2023-11-10T17:49:21Z",
"sourceType": "Kustomize",
"summary": {
"images": [
"123456478901.dkr.ecr.us-east-1.amazonaws.com/argo-diff:pr-1"
]
},
"controllerNamespace": "argocd"
}
}
]
}
22 changes: 14 additions & 8 deletions internal/argocd/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,39 @@ func filterApplications(a []v1alpha1.Application, repoOwner, repoName, repoDefau
ghMatch2 := fmt.Sprintf("github.com/%s/%s", repoOwner, repoName)
log.Debug().Msgf("filterApplications() - matching candidates against '%s' and '%s'", ghMatch1, ghMatch2)
for _, app := range a {
if !strings.HasSuffix(app.Spec.Source.RepoURL, ghMatch1) && !strings.HasSuffix(app.Spec.Source.RepoURL, ghMatch2) {
log.Debug().Msgf("Filtering application %s: RepoURL %s doesn't much %s or %s", app.ObjectMeta.Name, app.Spec.Source.RepoURL, ghMatch1, ghMatch2)
if len(app.Spec.Sources) > 0 {
log.Info().Msgf("Application %s has multiple sources - skipping as it is not supported", app.ObjectMeta.Name)
continue
}
appSpecSource := app.Spec.GetSource()

if !strings.HasSuffix(appSpecSource.RepoURL, ghMatch1) && !strings.HasSuffix(appSpecSource.RepoURL, ghMatch2) {
log.Debug().Msgf("Filtering application %s: RepoURL %s doesn't much %s or %s", app.ObjectMeta.Name, appSpecSource.RepoURL, ghMatch1, ghMatch2)
continue
}
if baseRef != "" {
// Processing a PR ...
if app.Spec.Source.TargetRevision == "HEAD" && baseRef != repoDefaultRef {
if appSpecSource.TargetRevision == "HEAD" && baseRef != repoDefaultRef {
// filter application if argo targets repo default (eg: main) and PR is not targetting main
log.Debug().Msgf("Filtering application %s: Target Rev is HEAD; baseRef %s != repoDefaultRef %s", app.ObjectMeta.Name, baseRef, repoDefaultRef)
continue
}
if app.Spec.Source.TargetRevision != "HEAD" && baseRef != app.Spec.Source.TargetRevision {
if appSpecSource.TargetRevision != "HEAD" && baseRef != appSpecSource.TargetRevision {
// filter application if argo doesn't target repo default (eg: main) and PR is not targetting that branch
log.Debug().Msgf("Filtering application %s: baseRef %s != Target Rev %s", app.ObjectMeta.Name, baseRef, app.Spec.Source.TargetRevision)
log.Debug().Msgf("Filtering application %s: baseRef %s != Target Rev %s", app.ObjectMeta.Name, baseRef, appSpecSource.TargetRevision)
continue
}
} else {
// processing a push
// eg: refs/heads/main -> main
changeRef = strings.TrimPrefix(changeRef, "refs/heads/")
// filter out apps where auto-sync is enabled for the branch of the push
if app.Spec.Source.TargetRevision == "HEAD" && changeRef == repoDefaultRef && app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.Automated != nil {
if appSpecSource.TargetRevision == "HEAD" && changeRef == repoDefaultRef && app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.Automated != nil {
log.Debug().Msgf("Filtering auto-sync application %s: Target Rev is HEAD; changeRef %s == repoDefaultRef %s", app.ObjectMeta.Name, changeRef, repoDefaultRef)
continue
}
if app.Spec.Source.TargetRevision != "HEAD" && changeRef == app.Spec.Source.TargetRevision && app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.Automated != nil {
log.Debug().Msgf("Filtering auto-sync application %s: changeRef %s = Target Rev %s", app.ObjectMeta.Name, changeRef, app.Spec.Source.TargetRevision)
if appSpecSource.TargetRevision != "HEAD" && changeRef == appSpecSource.TargetRevision && app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.Automated != nil {
log.Debug().Msgf("Filtering auto-sync application %s: changeRef %s = Target Rev %s", app.ObjectMeta.Name, changeRef, appSpecSource.TargetRevision)
continue
}
}
Expand Down

0 comments on commit 25e9797

Please sign in to comment.