refactor to support dynamic github app tokens (#428)

Author: djeebus

cmd/controller.go

@@ -18,7 +18,6 @@ import (
 	"github.com/zapier/kubechecks/pkg/config"
 	"github.com/zapier/kubechecks/pkg/container"
 	"github.com/zapier/kubechecks/pkg/events"
-	"github.com/zapier/kubechecks/pkg/git"
 	"github.com/zapier/kubechecks/pkg/server"
 	"github.com/zapier/kubechecks/telemetry"
 )
@@ -64,11 +63,6 @@ var ControllerCmd = &cobra.Command{
 			log.Info().Msgf("not monitoring applications, MonitorAllApplications: %+v", cfg.MonitorAllApplications)
 		}
 
-		log.Info().Msg("initializing git settings")
-		if err = initializeGit(ctr); err != nil {
-			log.Fatal().Err(err).Msg("failed to initialize git settings")
-		}
-
 		log.Info().Strs("locations", cfg.PoliciesLocation).Msg("processing policies locations")
 		if err = processLocations(ctx, ctr, cfg.PoliciesLocation); err != nil {
 			log.Fatal().Err(err).Msg("failed to process policy locations")
@@ -113,14 +107,6 @@ func startWebserver(ctx context.Context, ctr container.Container, processors []c
 	go srv.Start(ctx)
 }
 
-func initializeGit(ctr container.Container) error {
-	if err := git.SetCredentials(ctr.Config, ctr.VcsClient); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func waitForPendingRequest() {
 	for events.GetInFlight() > 0 {
 		log.Info().Int("count", events.GetInFlight()).Msg("waiting for in-flight requests to complete")

cmd/process.go

@@ -57,11 +57,6 @@ var processCmd = &cobra.Command{
 			log.Fatal().Err(err).Msg("failed to create clients")
 		}
 
-		log.Info().Msg("initializing git settings")
-		if err = initializeGit(ctr); err != nil {
-			log.Fatal().Err(err).Msg("failed to initialize git settings")
-		}
-
 		repo, err := ctr.VcsClient.LoadHook(ctx, args[0])
 		if err != nil {
 			log.Fatal().Err(err).Msg("failed to load hook")

pkg/config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"time"
@@ -126,6 +127,10 @@ func NewWithViper(v *viper.Viper) (ServerConfig, error) {
 		return cfg, errors.Wrap(err, "failed to read configuration")
 	}
 
+	if cfg.VcsBaseUrl == "" {
+		cfg.VcsBaseUrl = fmt.Sprintf("https://%s.com", cfg.VcsType)
+	}
+
 	log.Info().Msg("Server Configuration: ")
 	log.Info().Msgf("Webhook URL Base: %s", cfg.WebhookUrlBase)
 	log.Info().Msgf("Webhook URL Prefix: %s", cfg.UrlPrefix)

pkg/container/main.go

@@ -9,6 +9,7 @@ import (
 	client "github.com/zapier/kubechecks/pkg/kubernetes"
 	"github.com/zapier/kubechecks/pkg/vcs/github_client"
 	"github.com/zapier/kubechecks/pkg/vcs/gitlab_client"
+	"go.opentelemetry.io/otel"
 
 	"github.com/zapier/kubechecks/pkg/appdir"
 	"github.com/zapier/kubechecks/pkg/argo_client"
@@ -17,6 +18,8 @@ import (
 	"github.com/zapier/kubechecks/pkg/vcs"
 )
 
+var tracer = otel.Tracer("pkg/container")
+
 type Container struct {
 	ArgoClient *argo_client.ArgoClient
 
@@ -36,6 +39,9 @@ type ReposCache interface {
 }
 
 func New(ctx context.Context, cfg config.ServerConfig) (Container, error) {
+	ctx, span := tracer.Start(ctx, "New")
+	defer span.End()
+
 	var err error
 
 	var ctr = Container{
@@ -46,9 +52,9 @@ func New(ctx context.Context, cfg config.ServerConfig) (Container, error) {
 	// create vcs client
 	switch cfg.VcsType {
 	case "gitlab":
-		ctr.VcsClient, err = gitlab_client.CreateGitlabClient(cfg)
+		ctr.VcsClient, err = gitlab_client.CreateGitlabClient(ctx, cfg)
 	case "github":
-		ctr.VcsClient, err = github_client.CreateGithubClient(cfg)
+		ctr.VcsClient, err = github_client.CreateGithubClient(ctx, cfg)
 	default:
 		err = fmt.Errorf("unknown vcs-type: %q", cfg.VcsType)
 	}

pkg/git/repo.go

@@ -3,9 +3,7 @@ package git
 import (
 	"bufio"
 	"context"
-	"encoding/json"
 	"fmt"
-	"io"
 	"io/fs"
 	"net/http"
 	"net/url"
@@ -14,7 +12,6 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
@@ -23,7 +20,6 @@ import (
 
 	"github.com/zapier/kubechecks/pkg"
 	"github.com/zapier/kubechecks/pkg/config"
-	"github.com/zapier/kubechecks/pkg/vcs"
 	"github.com/zapier/kubechecks/telemetry"
 )
 
@@ -213,7 +209,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, ref string) error {
 			attribute.String("sha", ref),
 		))
 	defer span.End()
-	merge_command := []string{"merge", ref}
+	mergeCommand := []string{"merge", ref}
 	// For shallow clones, we need to pull the ref into the repo
 	if r.Shallow {
 		ref = strings.TrimPrefix(ref, "origin/")
@@ -227,10 +223,10 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, ref string) error {
 		// When merging shallow clones, we need to allow unrelated histories
 		// and use the "theirs" strategy to avoid conflicts
 		// cons of this is that it may not be entirely accurate and may overwrite changes in the target branch
-		merge_command = []string{"merge", ref, "--allow-unrelated-histories", "-X", "theirs"}
+		mergeCommand = []string{"merge", ref, "--allow-unrelated-histories", "-X", "theirs"}
 	}
 
-	cmd := r.execGitCommand(merge_command...)
+	cmd := r.execGitCommand(mergeCommand...)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		telemetry.SetError(span, err, "merge commit into branch")
@@ -298,10 +294,9 @@ func censorVcsToken(cfg config.ServerConfig, args []string) []string {
 }
 
 // SetCredentials ensures Git auth is set up for cloning
-func SetCredentials(cfg config.ServerConfig, vcsClient vcs.Client) error {
-	email := vcsClient.Email()
-	username := vcsClient.Username()
-	cloneUsername := vcsClient.CloneUsername()
+func SetCredentials(ctx context.Context, cfg config.ServerConfig, email, username, cloneUrl string) error {
+	_, span := tracer.Start(ctx, "SetCredentials")
+	defer span.End()
 
 	cmd := execCommand(cfg, "git", "config", "--global", "user.email", email)
 	err := cmd.Run()
@@ -315,14 +310,6 @@ func SetCredentials(cfg config.ServerConfig, vcsClient vcs.Client) error {
 		return errors.Wrap(err, "failed to set git user name")
 	}
 
-	httpClient := &http.Client{
-		Timeout: 30 * time.Second,
-	}
-	cloneUrl, err := getCloneUrl(cloneUsername, cfg, httpClient)
-	if err != nil {
-		return errors.Wrap(err, "failed to get clone url")
-	}
-
 	homedir, err := os.UserHomeDir()
 	if err != nil {
 		return errors.Wrap(err, "unable to get home directory")
@@ -350,73 +337,17 @@ func SetCredentials(cfg config.ServerConfig, vcsClient vcs.Client) error {
 	return nil
 }
 
-func getCloneUrl(user string, cfg config.ServerConfig, httpClient HTTPClient) (string, error) {
-	vcsBaseUrl := cfg.VcsBaseUrl
-	vcsType := cfg.VcsType
-	vcsToken := cfg.VcsToken
-
+func BuildCloneURL(baseURL, user, password string) (string, error) {
 	var hostname, scheme string
 
-	if vcsBaseUrl == "" {
-		// hack: but it does happen to work for now
-		hostname = fmt.Sprintf("%s.com", vcsType)
-		scheme = "https"
-	} else {
-		parts, err := url.Parse(vcsBaseUrl)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to parse %q", vcsBaseUrl)
-		}
-		hostname = parts.Host
-		scheme = parts.Scheme
-	}
-
-	if cfg.IsGithubApp() {
-		stringAppId := fmt.Sprintf("%d", cfg.GithubAppID)
-		jwt, err := pkg.CreateJWT(cfg.GithubPrivateKey, stringAppId)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to create jwt")
-		}
-		url := fmt.Sprintf("https://api.github.com/app/installations/%d/access_tokens", cfg.GithubInstallationID)
-
-		req, err := http.NewRequest(http.MethodPost, url, nil)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to create request")
-		}
-		req.Header.Add("Accept", "application/vnd.github.v3+json")
-		req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", jwt))
-
-		resp, err := httpClient.Do(req)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to get response")
-		}
-		defer func() {
-			if closeErr := resp.Body.Close(); closeErr != nil {
-				log.Warn().Err(closeErr).Msg("failed to close response body")
-			}
-		}()
-
-		body, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to read response")
-		}
-
-		var result interface{}
-		err = json.Unmarshal(body, &result)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to unmarshal response")
-		}
-
-		data, ok := result.(map[string]interface{})
-		if !ok {
-			return "", errors.New("failed to convert response to map")
-		}
-
-		if token, ok := data["token"].(string); ok {
-			vcsToken = token
-		}
+	parts, err := url.Parse(baseURL)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to parse %q", baseURL)
 	}
+	hostname = parts.Host
+	scheme = parts.Scheme
 
-	return fmt.Sprintf("%s://%s:%s@%s", scheme, user, vcsToken, hostname), nil
+	return fmt.Sprintf("%s://%s:%s@%s", scheme, user, password, hostname), nil
 }
 
 // GetListOfChangedFiles returns a list of files that have changed between the current branch and the target branch