Skip to content
Draft
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
5 changes: 4 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ run:
build-tags:
- e2e
linters-settings:
govet:
disable:
- inline
gocritic:
disabled-checks:
- unlambda
Expand Down Expand Up @@ -50,7 +53,7 @@ linters:
# - execinquery
- exhaustive
#- exhaustruct
- exportloopref
- copyloopvar
- forbidigo
- forcetypeassert
#- funlen
Expand Down
8 changes: 7 additions & 1 deletion .tekton/go.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ spec:
value: $(workspaces.source.path)/go-build-cache/cache
- name: GOMODCACHE
value: $(workspaces.source.path)/go-build-cache/mod
- name: GOTOOLCHAIN
value: go1.21.13
workingDir: $(workspaces.source.path)
script: |
#!/usr/bin/env bash
Expand All @@ -81,6 +83,8 @@ spec:
value: $(workspaces.source.path)/go-build-cache/cache
- name: GOMODCACHE
value: $(workspaces.source.path)/go-build-cache/mod
- name: GOTOOLCHAIN
value: go1.21.13
- name: GITHUB_REPOSITORY
value: "{{repo_owner}}/{{repo_name}}"
- name: GITHUB_PULL_REQUEST_ID
Expand All @@ -96,13 +100,15 @@ spec:
chmod +x ./codecov
./codecov -P $GITHUB_PULL_REQUEST_ID -C {{revision}} -v
- name: lint
image: golangci/golangci-lint:latest
image: golang:1.21
workingDir: $(workspaces.source.path)
env:
- name: GOCACHE
value: $(workspaces.source.path)/go-build-cache/cache
- name: GOMODCACHE
value: $(workspaces.source.path)/go-build-cache/mod
- name: GOTOOLCHAIN
value: go1.21.13
- name: GOLANGCILINT_CACHE
value: $(workspaces.source.path)/go-build-cache/golangci-cache
script: |
Expand Down
28 changes: 23 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
TARGET_NAMESPACE=pipelines-as-code
GOLANGCI_LINT=golangci-lint
GOLANGCI_LINT_VERSION ?= v1.59.1
GO_TOOLCHAIN ?= go1.21.13
GOFUMPT=gofumpt
TKN_BINARY_NAME := tkn
TKN_BINARY_URL := https://tekton.dev/docs/cli/\#installation
Expand All @@ -9,7 +10,18 @@ GO = go
TIMEOUT_UNIT = 20m
TIMEOUT_E2E = 45m
GO_TEST_FLAGS +=
GOTOOLCHAIN ?= $(GO_TOOLCHAIN)
export GOTOOLCHAIN
SHELL := bash
TOPDIR := $(shell git rev-parse --show-toplevel)
TMPDIR := $(TOPDIR)/tmp
GOLANGCI_LINT_OS ?= $(shell uname -s | tr '[:upper:]' '[:lower:]')
GOLANGCI_LINT_ARCH ?= $(shell uname -m | sed -e 's/x86_64/amd64/' -e 's/aarch64/arm64/')
GOLANGCI_LINT_PACKAGE := golangci-lint-$(patsubst v%,%,$(GOLANGCI_LINT_VERSION))-$(GOLANGCI_LINT_OS)-$(GOLANGCI_LINT_ARCH)
GOLANGCI_LINT_DIR := $(TMPDIR)/golangci-lint/$(GOLANGCI_LINT_VERSION)
GOLANGCI_LINT_BIN := $(GOLANGCI_LINT_DIR)/golangci-lint
GOLANGCI_LINT ?= $(GOLANGCI_LINT_BIN)
GOLANGCI_LINT_EXTRA_ARGS ?= --concurrency=1

PY_FILES := $(shell find . -type f -regex ".*\.py" -print)
SH_FILES := $(shell find hack/ -type f -regex ".*\.sh" -print)
Expand Down Expand Up @@ -80,13 +92,21 @@ html-coverage: ## generate html coverage
lint: lint-go lint-yaml lint-md lint-python lint-shell ## run all linters

.PHONY: lint-go
lint-go: ## runs go linter on all go files
lint-go: golangci-lint ## runs go linter on all go files
@echo "Linting go files..."
@$(GOLANGCI_LINT) run ./... --modules-download-mode=vendor \
@$(GOLANGCI_LINT) run $(GOLANGCI_LINT_EXTRA_ARGS) ./... --modules-download-mode=vendor \
--max-issues-per-linter=0 \
--max-same-issues=0 \
--timeout $(TIMEOUT_UNIT)

.PHONY: golangci-lint
golangci-lint: $(GOLANGCI_LINT_BIN) ## download pinned golangci-lint into tmp

$(GOLANGCI_LINT_BIN):
@mkdir -p $(GOLANGCI_LINT_DIR)
@echo "Downloading golangci-lint $(GOLANGCI_LINT_VERSION) for $(GOLANGCI_LINT_OS)-$(GOLANGCI_LINT_ARCH)"
@curl -fsSL "https://github.com/golangci/golangci-lint/releases/download/$(GOLANGCI_LINT_VERSION)/$(GOLANGCI_LINT_PACKAGE).tar.gz" | tar -xz -C "$(GOLANGCI_LINT_DIR)" --strip-components=1 "$(GOLANGCI_LINT_PACKAGE)/golangci-lint"

.PHONY: lint-yaml
lint-yaml: ${YAML_FILES} ## runs yamllint on all yaml files
@echo "Linting yaml files..."
Expand Down Expand Up @@ -192,5 +212,3 @@ generated: update-golden fumpt ## generate all files that needs to be generated
.PHONY: clean
clean: ## clean build artifacts
rm -fR bin


2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ require (

require (
github.com/coreos/go-oidc/v3 v3.9.0 // indirect
github.com/go-jose/go-jose/v3 v3.0.3 // indirect
github.com/go-jose/go-jose/v3 v3.0.5 // indirect
)

replace (
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,8 @@ github.com/go-fonts/stix v0.1.0/go.mod h1:w/c1f0ldAUlJmLBvlbkvVXLAD+tAMqobIIQpmn
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-jose/go-jose/v3 v3.0.3 h1:fFKWeig/irsp7XD2zBxvnmA/XaRWp5V3CBsZXJF7G7k=
github.com/go-jose/go-jose/v3 v3.0.3/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ=
github.com/go-jose/go-jose/v3 v3.0.5 h1:BLLJWbC4nMZOfuPVxoZIxeYsn6Nl2r1fITaJ78UQlVQ=
github.com/go-jose/go-jose/v3 v3.0.5/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ=
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
Expand Down
3 changes: 2 additions & 1 deletion pkg/adapter/incoming.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (l *listener) detectIncoming(ctx context.Context, req *http.Request, payloa
if req.URL.Path != "/incoming" {
return false, nil, nil
}
l.logger.Infof("incoming request has been requested: %v", req.URL)
l.logger.Infof("incoming request has been requested: %v", req.URL.Path)
if pipelineRun == "" || repository == "" || querySecret == "" || branch == "" {
err := fmt.Errorf("missing query URL argument: pipelinerun, branch, repository, secret: '%s' '%s' '%s' '%s'", pipelineRun, branch, repository, querySecret)
return false, nil, err
Expand Down Expand Up @@ -112,6 +112,7 @@ func (l *listener) detectIncoming(ctx context.Context, req *http.Request, payloa
return false, nil, err
}
l.event.Provider.URL = enterpriseURL
l.event.GHEURL = enterpriseURL
l.event.Provider.Token = token
l.event.InstallationID = installationID
// Github app is not installed for provided repository url
Expand Down
12 changes: 10 additions & 2 deletions pkg/adapter/incoming_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package adapter

import (
"context"
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
"reflect"
"strings"
Expand Down Expand Up @@ -33,6 +34,13 @@ import (
"github.com/openshift-pipelines/pipelines-as-code/pkg/test/kubernetestint"
)

func newRequest(t *testing.T, method, target string, body io.Reader) *http.Request {
t.Helper()
req, err := http.NewRequestWithContext(context.Background(), method, target, body)
assert.NilError(t, err)
return req
}

const fakePrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQC6GorZBeri0eVERMZQDFh5E1RMPjFk9AevaWr27yJse6eiUlos
gY2L2vcZKLOrdvVR+TLLapIMFfg1E1qVr1iTHP3IiSCs1uW6NKDmxEQc9Uf/fG9c
Expand Down Expand Up @@ -682,7 +690,7 @@ func Test_listener_detectIncoming(t *testing.T) {
}

// make a new request
req := httptest.NewRequest(tt.args.method,
req := newRequest(t, tt.args.method,
fmt.Sprintf("http://localhost%s?repository=%s&secret=%s&pipelinerun=%s&branch=%s", tt.args.queryURL,
tt.args.queryRepository, tt.args.querySecret, tt.args.queryPipelineRun, tt.args.queryBranch),
strings.NewReader(tt.args.incomingBody))
Expand Down
7 changes: 1 addition & 6 deletions pkg/adapter/sinker.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,7 @@ func (s *sinker) processEventPayload(ctx context.Context, request *http.Request)
}

func (s *sinker) processEvent(ctx context.Context, request *http.Request) error {
if s.event.EventType == "incoming" {
if request.Header.Get("X-GitHub-Enterprise-Host") != "" {
s.event.Provider.URL = request.Header.Get("X-GitHub-Enterprise-Host")
s.event.GHEURL = request.Header.Get("X-GitHub-Enterprise-Host")
}
} else {
if s.event.EventType != "incoming" {
if err := s.processEventPayload(ctx, request); err != nil {
return err
}
Expand Down
20 changes: 16 additions & 4 deletions pkg/provider/github/app/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import (
"context"
"fmt"
"net/http"
"net/url"
"strings"
"time"

"github.com/golang-jwt/jwt/v4"
gt "github.com/google/go-github/v61/github"
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github"
Expand Down Expand Up @@ -47,11 +50,20 @@ func (ip *Install) GetAndUpdateInstallationID(ctx context.Context) (string, stri
return "", "", 0, err
}

if ip.ghClient.APIURL == nil {
return "", "", 0, fmt.Errorf("github client APIURL is nil")
}
Comment on lines +53 to +55
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

A potential nil pointer dereference can occur if ip.ghClient is nil. Accessing ip.ghClient.APIURL directly will cause a panic. We should guard against ip.ghClient being nil before checking ip.ghClient.APIURL.

Suggested change
if ip.ghClient.APIURL == nil {
return "", "", 0, fmt.Errorf("github client APIURL is nil")
}
if ip.ghClient == nil || ip.ghClient.APIURL == nil {
return "", "", 0, fmt.Errorf("github client or its APIURL is nil")
}

apiURL := *ip.ghClient.APIURL
enterpriseHost = ip.request.Header.Get("X-GitHub-Enterprise-Host")
if enterpriseHost != "" {
// NOTE: Hopefully this works even when the ghe URL is on another host than the api URL
apiURL = "https://" + enterpriseHost + "/api/v3"
repoURL, err := url.Parse(ip.repo.Spec.URL)
if err != nil {
return "", "", 0, fmt.Errorf("failed to parse repository URL: %w", err)
}
enterpriseHost = ""
if repoURL.Host != "" && repoURL.Host != "github.com" {
enterpriseHost = repoURL.Host
if apiURL == keys.PublicGithubAPIURL {
apiURL = fmt.Sprintf("https://%s/api/v3", strings.TrimSuffix(enterpriseHost, "/"))
}
}

logger := logging.FromContext(ctx)
Expand Down
93 changes: 88 additions & 5 deletions pkg/provider/github/app/token_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package app

import (
"context"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"

Expand All @@ -22,6 +23,13 @@ import (
rtesting "knative.dev/pkg/reconciler/testing"
)

func newLocalhostGetRequest(t *testing.T, body io.Reader) *http.Request {
t.Helper()
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://localhost", body)
assert.NilError(t, err)
return req
}

const fakePrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQC6GorZBeri0eVERMZQDFh5E1RMPjFk9AevaWr27yJse6eiUlos
gY2L2vcZKLOrdvVR+TLLapIMFfg1E1qVr1iTHP3IiSCs1uW6NKDmxEQc9Uf/fG9c
Expand Down Expand Up @@ -144,7 +152,7 @@ func Test_GenerateJWT(t *testing.T) {
},
}

ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, tt.namespace.GetName())
ip := NewInstallation(newLocalhostGetRequest(t, strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, tt.namespace.GetName())
token, err := ip.GenerateJWT(ctx)
if tt.wantErr {
assert.Assert(t, err != nil)
Expand Down Expand Up @@ -205,10 +213,10 @@ func Test_GetAndUpdateInstallationID(t *testing.T) {
ctx = info.StoreCurrentControllerName(ctx, "default")
ctx = info.StoreNS(ctx, testNamespace.GetName())

ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName())
ip := NewInstallation(newLocalhostGetRequest(t, strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName())
jwtToken, err := ip.GenerateJWT(ctx)
assert.NilError(t, err)
req := httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader(""))
req := newLocalhostGetRequest(t, strings.NewReader(""))
repo := &v1alpha1.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo",
Expand Down Expand Up @@ -256,6 +264,81 @@ func Test_GetAndUpdateInstallationID(t *testing.T) {
assert.Equal(t, token, wantToken)
}

func TestGetAndUpdateInstallationIDIgnoresEnterpriseHostHeader(t *testing.T) {
tdata := testclient.Data{
Namespaces: []*corev1.Namespace{testNamespace},
Secret: []*corev1.Secret{validSecret},
}
wantToken := "GOODTOKEN"
wantID := 120
orgName := "org"
repoName := "repo"

fakeghclient, mux, serverURL, teardown := ghtesthelper.SetupGH()
defer teardown()

ctx, _ := rtesting.SetupFakeContext(t)
stdata, _ := testclient.SeedTestData(t, ctx, tdata)
logger, _ := logger.GetLogger()
run := &params.Run{
Clients: clients.Clients{
Log: logger,
PipelineAsCode: stdata.PipelineAsCode,
Kube: stdata.Kube,
},
Info: info.Info{
Pac: &info.PacOpts{
Settings: settings.Settings{},
},
Controller: &info.ControllerInfo{Secret: validSecret.GetName()},
},
}
ctx = info.StoreCurrentControllerName(ctx, "default")
ctx = info.StoreNS(ctx, testNamespace.GetName())

jwtInstall := NewInstallation(newLocalhostGetRequest(t, strings.NewReader("")), run, &v1alpha1.Repository{}, &github.Provider{}, testNamespace.GetName())
jwtToken, err := jwtInstall.GenerateJWT(ctx)
assert.NilError(t, err)

mux.HandleFunc("/app/installations", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Authorization", "Bearer 12345")
w.Header().Set("Accept", "application/vnd.github+json")
_, _ = fmt.Fprintf(w, `[{"id":%d}]`, wantID)
})
mux.HandleFunc(fmt.Sprintf("/app/installations/%d/access_tokens", wantID), func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.Header().Set("Authorization", "Bearer "+jwtToken)
w.Header().Set("Accept", "application/vnd.github+json")
_, _ = fmt.Fprintf(w, `{"token": "%s"}`, wantToken)
})
mux.HandleFunc("/installation/repositories", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Authorization", "Bearer 12345")
w.Header().Set("Accept", "application/vnd.github+json")
_, _ = fmt.Fprintf(w, `{"total_count": 1,"repositories": [{"id":1,"html_url": "https://github.com/%s/%s"}]}`, orgName, repoName)
})

repo := &v1alpha1.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo",
},
Spec: v1alpha1.RepositorySpec{
URL: fmt.Sprintf("https://github.com/%s/%s", orgName, repoName),
},
}

gprovider := &github.Provider{Client: fakeghclient, APIURL: &serverURL, Run: run}
t.Setenv("PAC_GIT_PROVIDER_TOKEN_APIURL", serverURL+"/api/v3")

req := newLocalhostGetRequest(t, strings.NewReader(""))
req.Header.Set("X-GitHub-Enterprise-Host", "127.0.0.1:1")
ip := NewInstallation(req, run, repo, gprovider, testNamespace.GetName())
enterpriseURL, token, installationID, err := ip.GetAndUpdateInstallationID(ctx)
assert.NilError(t, err)
assert.Equal(t, enterpriseURL, "")
assert.Equal(t, installationID, int64(wantID))
assert.Equal(t, token, wantToken)
}

func testMethod(t *testing.T, r *http.Request, want string) {
t.Helper()
if got := r.Method; got != want {
Expand Down Expand Up @@ -296,7 +379,7 @@ func Test_ListRepos(t *testing.T) {

ctx, _ := rtesting.SetupFakeContext(t)
gprovider := &github.Provider{Client: fakeclient}
ip := NewInstallation(httptest.NewRequest(http.MethodGet, "http://localhost", strings.NewReader("")),
ip := NewInstallation(newLocalhostGetRequest(t, strings.NewReader("")),
&params.Run{}, repo, gprovider, testNamespace.GetName())
exist, err := ip.matchRepos(ctx)
assert.NilError(t, err)
Expand Down
Loading
Loading