From 742717a7201708f2d2d2db330e19316b94f56f21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 2 Jan 2025 10:42:29 +0100 Subject: [PATCH 01/16] Add support for Jenkins --- cmd/image-builder/config.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 0b9e71c489be..5fc3daea8644 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -326,6 +326,11 @@ func loadGithubActionsGitState() (GitStateConfig, error) { } } +func loadJenksingGitState() (GitStateConfig, error) { + // Load from env specific for Jenkins Jobs + return GitStateConfig{}, fmt.Errorf("Jenkins is not supported as CI system") +} + // DetermineUsedCISystem return CISystem bind to system in which image builder is running or error if unknown // It is used to avoid getting env variables in multiple parts of image builder func DetermineUsedCISystem() (CISystem, error) { From 9a400fd0ad94099fedd63568c8cfd44247099121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 2 Jan 2025 11:02:36 +0100 Subject: [PATCH 02/16] Add Jenkins --- cmd/image-builder/config.go | 9 +++++++++ cmd/image-builder/config_test.go | 7 +++++++ go.mod | 3 ++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 5fc3daea8644..839b56e6b748 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -21,6 +21,7 @@ const ( Prow CISystem = "Prow" GithubActions CISystem = "GithubActions" AzureDevOps CISystem = "AzureDevOps" + Jenkins CISystem = "Jenkins" ) type Config struct { @@ -155,6 +156,8 @@ func LoadGitStateConfig(ciSystem CISystem) (GitStateConfig, error) { // Load from env specific for Github Actions case GithubActions: return loadGithubActionsGitState() + case Jenkins: + return loadJenksingGitState() default: // Unknown CI System, return error and empty git state return GitStateConfig{}, fmt.Errorf("unknown ci system, got %s", ciSystem) @@ -363,5 +366,11 @@ func determineUsedCISystem(envGetter func(key string) string, envLookup func(key return AzureDevOps, nil } + // JENKINS_HOME environment variable is set in Jenkins + _, isJenkins := envLookup("JENKINS_HOME") + if isJenkins { + return Jenkins, nil + } + return "", fmt.Errorf("cannot determine ci system: unknown system") } diff --git a/cmd/image-builder/config_test.go b/cmd/image-builder/config_test.go index 97ae9d27aa2f..a54cb4480122 100644 --- a/cmd/image-builder/config_test.go +++ b/cmd/image-builder/config_test.go @@ -436,6 +436,13 @@ func Test_determineCISystem(t *testing.T) { }, ciSystem: GithubActions, }, + { + name: "detect running in jenkins", + env: mockEnv{ + "JENKINS_HOME": "/some/absolute/path", + }, + ciSystem: Jenkins, + }, { name: "unknown ci system", env: mockEnv{ diff --git a/go.mod b/go.mod index 39663de4e277..08d8f63a64c8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,7 @@ module github.com/kyma-project/test-infra -go 1.22.0 +go 1.23 + toolchain go1.23.0 require ( From 195fe5d3654f7634cac14b06a0fd4fb31e87f739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 2 Jan 2025 11:23:36 +0100 Subject: [PATCH 03/16] Update to go 1.23 --- cmd/oidc-token-verifier/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oidc-token-verifier/Dockerfile b/cmd/oidc-token-verifier/Dockerfile index 7d7690f2a4d7..4c150865c382 100644 --- a/cmd/oidc-token-verifier/Dockerfile +++ b/cmd/oidc-token-verifier/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.22.3-alpine as builder +FROM golang:1.23-alpine as builder # Add certificate authorities certificates. # This let oidctokenverifier to use tls connection to the OIDC provider. From 81b16b7c270f7f8a5fbdc8ea97e3aab17e9215dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 2 Jan 2025 12:11:00 +0100 Subject: [PATCH 04/16] Parse jenkins env --- cmd/image-builder/config.go | 53 ++++++++++++++++++++++++++++++-- cmd/image-builder/config_test.go | 42 +++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 839b56e6b748..8f1fce74890f 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "regexp" "slices" "strconv" @@ -157,7 +158,7 @@ func LoadGitStateConfig(ciSystem CISystem) (GitStateConfig, error) { case GithubActions: return loadGithubActionsGitState() case Jenkins: - return loadJenksingGitState() + return loadJenkinsGitState() default: // Unknown CI System, return error and empty git state return GitStateConfig{}, fmt.Errorf("unknown ci system, got %s", ciSystem) @@ -329,9 +330,55 @@ func loadGithubActionsGitState() (GitStateConfig, error) { } } -func loadJenksingGitState() (GitStateConfig, error) { +func loadJenkinsGitState() (GitStateConfig, error) { // Load from env specific for Jenkins Jobs - return GitStateConfig{}, fmt.Errorf("Jenkins is not supported as CI system") + prID, isPullRequest := os.LookupEnv("CHANGE_ID") + gitURL := os.Getenv("GIT_URL") + + owner, repo, err := extractOwnerAndRepoFromGitURL(gitURL) + if err != nil { + return GitStateConfig{}, fmt.Errorf("failed to extract owner and repository from git URL %s: %w", gitURL, err) + } + + baseCommitSHA := os.Getenv("GIT_COMMIT") + + gitState := GitStateConfig{ + RepositoryName: repo, + RepositoryOwner: owner, + JobType: "postsubmit", + BaseCommitSHA: baseCommitSHA, + } + + if isPullRequest { + pullNumber, err := strconv.Atoi(prID) + if err != nil { + return GitStateConfig{}, fmt.Errorf("failed to parse CHANGE_ID environment variable: %w", err) + } + + baseRef := os.Getenv("CHANGE_BRANCH") + pullRequestHeadSHA := os.Getenv("CHANGE_TARGET") + + gitState.JobType = "presubmit" + gitState.PullRequestNumber = pullNumber + gitState.BaseCommitRef = baseRef + gitState.PullHeadCommitSHA = pullRequestHeadSHA + gitState.isPullRequest = true + } + + return gitState, nil +} + +func extractOwnerAndRepoFromGitURL(gitURL string) (string, string, error) { + re := regexp.MustCompile(`.*/(.*)/(.*)`) + + matches := re.FindStringSubmatch(gitURL) + fmt.Println(matches, gitURL) + + if len(matches) != 3 { + return "", "", fmt.Errorf("failed to extract owner and repository from git URL") + } + + return matches[1], matches[2], nil } // DetermineUsedCISystem return CISystem bind to system in which image builder is running or error if unknown diff --git a/cmd/image-builder/config_test.go b/cmd/image-builder/config_test.go index a54cb4480122..5d5325924886 100644 --- a/cmd/image-builder/config_test.go +++ b/cmd/image-builder/config_test.go @@ -379,6 +379,48 @@ func TestLoadGitStateConfig(t *testing.T) { PullHeadCommitSHA: "e47034172c36d3e5fb407b5ba57adf0f7868599d", }, }, + { + name: "load data from push event for jenkins", + options: options{ + ciSystem: Jenkins, + }, + env: map[string]string{ + "BRANCH_NAME": "refs/heads/main", + "JENKINS_HOME": "/some/absolute/path", + "GIT_URL": "github.com/kyma-project/test-infra", + "GIT_COMMIT": "1234", + "CHANGE_TARGET": "refs/heads/main", + }, + gitState: GitStateConfig{ + RepositoryName: "test-infra", + RepositoryOwner: "kyma-project", + JobType: "postsubmit", + BaseCommitSHA: "1234", + }, + }, + { + name: "load data from pull request event for jenkins", + options: options{ + ciSystem: Jenkins, + }, + env: map[string]string{ + "BRANCH_NAME": "refs/heads/main", + "JENKINS_HOME": "/some/absolute/path", + "CHANGE_ID": "14", + "GIT_URL": "github.com/kyma-project/test-infra", + "GIT_COMMIT": "1234", + "CHANGE_TARGET": "4321", + }, + gitState: GitStateConfig{ + RepositoryName: "test-infra", + RepositoryOwner: "kyma-project", + JobType: "presubmit", + BaseCommitSHA: "1234", + PullRequestNumber: 14, + PullHeadCommitSHA: "4321", + isPullRequest: true, + }, + }, } for _, c := range tc { From 44225f2212aa0dce310cecd78cf8b82106b18177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= Date: Thu, 2 Jan 2025 12:43:51 +0100 Subject: [PATCH 05/16] Rename env --- cmd/image-builder/config.go | 2 +- cmd/image-builder/config_test.go | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 8f1fce74890f..0855ad6f1cd1 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -356,7 +356,7 @@ func loadJenkinsGitState() (GitStateConfig, error) { } baseRef := os.Getenv("CHANGE_BRANCH") - pullRequestHeadSHA := os.Getenv("CHANGE_TARGET") + pullRequestHeadSHA := os.Getenv("CHANGE_HEAD_SHA") gitState.JobType = "presubmit" gitState.PullRequestNumber = pullNumber diff --git a/cmd/image-builder/config_test.go b/cmd/image-builder/config_test.go index 5d5325924886..49d91b9ad470 100644 --- a/cmd/image-builder/config_test.go +++ b/cmd/image-builder/config_test.go @@ -385,11 +385,10 @@ func TestLoadGitStateConfig(t *testing.T) { ciSystem: Jenkins, }, env: map[string]string{ - "BRANCH_NAME": "refs/heads/main", - "JENKINS_HOME": "/some/absolute/path", - "GIT_URL": "github.com/kyma-project/test-infra", - "GIT_COMMIT": "1234", - "CHANGE_TARGET": "refs/heads/main", + "BRANCH_NAME": "refs/heads/main", + "JENKINS_HOME": "/some/absolute/path", + "GIT_URL": "github.com/kyma-project/test-infra", + "GIT_COMMIT": "1234", }, gitState: GitStateConfig{ RepositoryName: "test-infra", @@ -404,12 +403,12 @@ func TestLoadGitStateConfig(t *testing.T) { ciSystem: Jenkins, }, env: map[string]string{ - "BRANCH_NAME": "refs/heads/main", - "JENKINS_HOME": "/some/absolute/path", - "CHANGE_ID": "14", - "GIT_URL": "github.com/kyma-project/test-infra", - "GIT_COMMIT": "1234", - "CHANGE_TARGET": "4321", + "BRANCH_NAME": "refs/heads/main", + "JENKINS_HOME": "/some/absolute/path", + "CHANGE_ID": "14", + "GIT_URL": "github.com/kyma-project/test-infra", + "GIT_COMMIT": "1234", + "CHANGE_HEAD_SHA": "4321", // Must be explicitly set when calling docker run }, gitState: GitStateConfig{ RepositoryName: "test-infra", From 2047f6cb56527ee275f359c28fa4a2e6233a76e1 Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 13:46:28 +0100 Subject: [PATCH 06/16] [Debug Print] Update config.go --- cmd/image-builder/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 0855ad6f1cd1..5ac1260c97ed 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -364,7 +364,9 @@ func loadJenkinsGitState() (GitStateConfig, error) { gitState.PullHeadCommitSHA = pullRequestHeadSHA gitState.isPullRequest = true } - + + log.Printf("JobType set to: %s", gitState.JobType) + return gitState, nil } From 860653b935a12dc28919606aab2d90cda46dff0c Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 13:50:19 +0100 Subject: [PATCH 07/16] [Debug Print] Update config.go --- cmd/image-builder/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 5ac1260c97ed..cfa4a6675443 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "log" "os" "regexp" "slices" From c6570a32be361ddfd1ccba8c498c3706d497d2e3 Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 14:05:21 +0100 Subject: [PATCH 08/16] [Debug Print] --- cmd/image-builder/config.go | 5 +---- cmd/image-builder/main.go | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index cfa4a6675443..0855ad6f1cd1 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "fmt" - "log" "os" "regexp" "slices" @@ -365,9 +364,7 @@ func loadJenkinsGitState() (GitStateConfig, error) { gitState.PullHeadCommitSHA = pullRequestHeadSHA gitState.isPullRequest = true } - - log.Printf("JobType set to: %s", gitState.JobType) - + return gitState, nil } diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index 351eacea1119..b82ffa5dc9a4 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -923,6 +923,7 @@ func main() { err = buildInADO(o) if err != nil { fmt.Printf("Image build failed with error: %s\n", err) + fmt.Printf(o.gitState.JobType) os.Exit(1) } os.Exit(0) From 49bd98a37f5cd601b55c53e07475bf0e9e5c6c8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20So=C5=82tys?= <74361703+Sawthis@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:16:53 +0100 Subject: [PATCH 09/16] Update .golangci.yaml --- .golangci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index 41705e83e6e5..8e6af74fa29b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -10,7 +10,7 @@ linters: disable-all: true enable: - unused - - exportloopref + - copyloopvar - gosimple - govet - ineffassign From 291f9b950dec090443da6145e4d5f626418ec5cb Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 14:25:00 +0100 Subject: [PATCH 10/16] [Debug Print] --- cmd/image-builder/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index b82ffa5dc9a4..e7f3ef3a1f34 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -923,7 +923,7 @@ func main() { err = buildInADO(o) if err != nil { fmt.Printf("Image build failed with error: %s\n", err) - fmt.Printf(o.gitState.JobType) + fmt.Printf("Git State: %v\n", o.gitState.JobType) os.Exit(1) } os.Exit(0) From 3fcdf291658252db2ec402835d5067ea4fd86148 Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 14:25:21 +0100 Subject: [PATCH 11/16] [Debug Print] --- cmd/image-builder/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index e7f3ef3a1f34..aaf36062a9f9 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -920,7 +920,9 @@ func main() { os.Exit(0) } if o.buildInADO { + fmt.Printf("Git State: %v\n", o.gitState.JobType) err = buildInADO(o) + fmt.Printf("Git State: %v\n", o.gitState.JobType) if err != nil { fmt.Printf("Image build failed with error: %s\n", err) fmt.Printf("Git State: %v\n", o.gitState.JobType) From b266e6ddcbf2e53c2919908dc1398c5ac9c0f146 Mon Sep 17 00:00:00 2001 From: Wojciech Soltys Date: Tue, 7 Jan 2025 14:41:53 +0100 Subject: [PATCH 12/16] The copy of the 'for' variable "change" can be deleted --- cmd/external-plugins/automated-approver/plugin.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/external-plugins/automated-approver/plugin.go b/cmd/external-plugins/automated-approver/plugin.go index cc45f057652a..4f4496a4ff53 100644 --- a/cmd/external-plugins/automated-approver/plugin.go +++ b/cmd/external-plugins/automated-approver/plugin.go @@ -183,7 +183,6 @@ func (ac *ApproveCondition) checkChangedFiles(logger *zap.SugaredLogger, changes defer logger.Sync() logger.Debugf("Checking if PR changed only allowed files: %v", ac.ChangedFiles) for _, change := range changes { - change := change logger.Debugf("Checking file: %s", change.Filename) matched := slices.ContainsFunc(ac.ChangedFiles, func(allowedFile string) bool { filesMatcher := regexp.MustCompile(allowedFile) From 960002193317270d56fcf49d232d83a0aadb576b Mon Sep 17 00:00:00 2001 From: Patryk Dobrowolski Date: Tue, 7 Jan 2025 14:42:38 +0100 Subject: [PATCH 13/16] [Debug Print] --- cmd/image-builder/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index aaf36062a9f9..bc045d9e86a6 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -920,12 +920,12 @@ func main() { os.Exit(0) } if o.buildInADO { - fmt.Printf("Git State: %v\n", o.gitState.JobType) + fmt.Printf("Git State1: %v\n", o.gitState.JobType) err = buildInADO(o) - fmt.Printf("Git State: %v\n", o.gitState.JobType) + fmt.Printf("Git State2: %v\n", o.gitState.JobType) if err != nil { fmt.Printf("Image build failed with error: %s\n", err) - fmt.Printf("Git State: %v\n", o.gitState.JobType) + fmt.Printf("Git State3: %v\n", o.gitState.JobType) os.Exit(1) } os.Exit(0) From 1d150188a7de18b8c31758eee1c7a609f4a5fae3 Mon Sep 17 00:00:00 2001 From: dekiel Date: Wed, 8 Jan 2025 16:38:47 +0100 Subject: [PATCH 14/16] Using logger in loadJenkinsGitState for better tracking. Added checking if required environment variables exists. Fixed regex to not match a .git suffix in reponame matching group. Using named matching groups for better readability. --- cmd/image-builder/config.go | 47 +++++++++++++++++++++++++------------ cmd/image-builder/main.go | 2 +- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index 0855ad6f1cd1..a16375368211 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -149,7 +149,7 @@ func (gitState GitStateConfig) IsPullRequest() bool { return gitState.isPullRequest } -func LoadGitStateConfig(ciSystem CISystem) (GitStateConfig, error) { +func LoadGitStateConfig(logger Logger, ciSystem CISystem) (GitStateConfig, error) { switch ciSystem { // Load from env specific for Azure DevOps and Prow Jobs case AzureDevOps, Prow: @@ -158,7 +158,7 @@ func LoadGitStateConfig(ciSystem CISystem) (GitStateConfig, error) { case GithubActions: return loadGithubActionsGitState() case Jenkins: - return loadJenkinsGitState() + return loadJenkinsGitState(logger) default: // Unknown CI System, return error and empty git state return GitStateConfig{}, fmt.Errorf("unknown ci system, got %s", ciSystem) @@ -330,17 +330,23 @@ func loadGithubActionsGitState() (GitStateConfig, error) { } } -func loadJenkinsGitState() (GitStateConfig, error) { +func loadJenkinsGitState(logger Logger) (GitStateConfig, error) { // Load from env specific for Jenkins Jobs prID, isPullRequest := os.LookupEnv("CHANGE_ID") - gitURL := os.Getenv("GIT_URL") + gitURL, present := os.LookupEnv("GIT_URL") + if !present { + return GitStateConfig{}, fmt.Errorf("GIT_URL environment variable is not set, please set it to valid git URL") + } - owner, repo, err := extractOwnerAndRepoFromGitURL(gitURL) + owner, repo, err := extractOwnerAndRepoFromGitURL(logger, gitURL) if err != nil { return GitStateConfig{}, fmt.Errorf("failed to extract owner and repository from git URL %s: %w", gitURL, err) } - baseCommitSHA := os.Getenv("GIT_COMMIT") + baseCommitSHA, present := os.LookupEnv("GIT_COMMIT") + if !present { + return GitStateConfig{}, fmt.Errorf("GIT_COMMIT environment variable is not set, please set it to valid commit SHA") + } gitState := GitStateConfig{ RepositoryName: repo, @@ -352,12 +358,17 @@ func loadJenkinsGitState() (GitStateConfig, error) { if isPullRequest { pullNumber, err := strconv.Atoi(prID) if err != nil { - return GitStateConfig{}, fmt.Errorf("failed to parse CHANGE_ID environment variable: %w", err) + return GitStateConfig{}, fmt.Errorf("failed to convert prID string variable to integer: %w", err) } - baseRef := os.Getenv("CHANGE_BRANCH") - pullRequestHeadSHA := os.Getenv("CHANGE_HEAD_SHA") - + baseRef, present := os.LookupEnv("CHANGE_BRANCH") + if !present { + return GitStateConfig{}, fmt.Errorf("CHANGE_BRANCH environment variable is not set, please set it to valid base branch name") + } + pullRequestHeadSHA, present := os.LookupEnv("CHANGE_HEAD_SHA") + if !present { + return GitStateConfig{}, fmt.Errorf("CHANGE_HEAD_SHA environment variable is not set, please set it to valid commit SHA") + } gitState.JobType = "presubmit" gitState.PullRequestNumber = pullNumber gitState.BaseCommitRef = baseRef @@ -368,17 +379,23 @@ func loadJenkinsGitState() (GitStateConfig, error) { return gitState, nil } -func extractOwnerAndRepoFromGitURL(gitURL string) (string, string, error) { - re := regexp.MustCompile(`.*/(.*)/(.*)`) - +func extractOwnerAndRepoFromGitURL(logger Logger, gitURL string) (string, string, error) { + re := regexp.MustCompile(`.*/(?P.*)/(?P.*).git`) matches := re.FindStringSubmatch(gitURL) - fmt.Println(matches, gitURL) + + logger.Debugw("Extracted matches from git URL", "matches", matches, "gitURL", gitURL) if len(matches) != 3 { return "", "", fmt.Errorf("failed to extract owner and repository from git URL") } - return matches[1], matches[2], nil + owner := matches[re.SubexpIndex("owner")] + repo := matches[re.SubexpIndex("repo")] + + logger.Debugw("Extracted owner from git URL", "owner", owner) + logger.Debugw("Extracted repository from git URL", "repo", repo) + + return owner, repo, nil } // DetermineUsedCISystem return CISystem bind to system in which image builder is running or error if unknown diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index bc045d9e86a6..a64972fa84e1 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -874,7 +874,7 @@ func main() { log.Fatalf("Failed to determine current ci system: %s", err) } o.ciSystem = ciSystem - o.gitState, err = LoadGitStateConfig(ciSystem) + o.gitState, err = LoadGitStateConfig(o.logger, ciSystem) if err != nil { log.Fatalf("Failed to load current git state: %s", err) } From 9165824dbb3a85181a7df39b40bdfb63e7d1211a Mon Sep 17 00:00:00 2001 From: dekiel Date: Wed, 8 Jan 2025 16:52:24 +0100 Subject: [PATCH 15/16] More logger usage. Deprecation warning. --- cmd/image-builder/config.go | 2 ++ cmd/image-builder/main.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/image-builder/config.go b/cmd/image-builder/config.go index a16375368211..d056f9631e55 100644 --- a/cmd/image-builder/config.go +++ b/cmd/image-builder/config.go @@ -149,6 +149,7 @@ func (gitState GitStateConfig) IsPullRequest() bool { return gitState.isPullRequest } +// TODO (dekiel): Add logger parameter to all functions reading a git state. func LoadGitStateConfig(logger Logger, ciSystem CISystem) (GitStateConfig, error) { switch ciSystem { // Load from env specific for Azure DevOps and Prow Jobs @@ -330,6 +331,7 @@ func loadGithubActionsGitState() (GitStateConfig, error) { } } +// loadJenkinsGitState loads git state from environment variables specific for Jenkins. func loadJenkinsGitState(logger Logger) (GitStateConfig, error) { // Load from env specific for Jenkins Jobs prID, isPullRequest := os.LookupEnv("CHANGE_ID") diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index a64972fa84e1..0c645c4848b0 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -869,14 +869,16 @@ func main() { // If running inside some CI system, determine which system is used if o.isCI { - ciSystem, err := DetermineUsedCISystem() + o.ciSystem, err = DetermineUsedCISystem() if err != nil { - log.Fatalf("Failed to determine current ci system: %s", err) + o.logger.Errorw("Failed to determine current ci system", "error", err) + os.Exit(1) } - o.ciSystem = ciSystem - o.gitState, err = LoadGitStateConfig(o.logger, ciSystem) + + o.gitState, err = LoadGitStateConfig(o.logger, o.ciSystem) if err != nil { - log.Fatalf("Failed to load current git state: %s", err) + o.logger.Errorw("Failed to load current git state", "error", err) + os.Exit(1) } o.logger.Debugw("Git state loaded", "gitState", o.gitState) @@ -920,17 +922,15 @@ func main() { os.Exit(0) } if o.buildInADO { - fmt.Printf("Git State1: %v\n", o.gitState.JobType) err = buildInADO(o) - fmt.Printf("Git State2: %v\n", o.gitState.JobType) if err != nil { - fmt.Printf("Image build failed with error: %s\n", err) - fmt.Printf("Git State3: %v\n", o.gitState.JobType) + o.logger.Errorw("Image build failed", "error", err, "JobType", o.gitState.JobType) os.Exit(1) } os.Exit(0) } + o.logger.Warnw("Local build is deprecated and will be removed soon, the tool will not support local building anymore. Please migrate to the ADO build backend.") err = buildLocally(o) if err != nil { fmt.Println(err) From d91396ee10c09d4d57b032e5df2a732fe68de7a9 Mon Sep 17 00:00:00 2001 From: dekiel Date: Wed, 8 Jan 2025 17:36:53 +0100 Subject: [PATCH 16/16] Fixed tests --- cmd/image-builder/config_test.go | 25 +++++++++++++++++-------- go.mod | 2 +- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/cmd/image-builder/config_test.go b/cmd/image-builder/config_test.go index 49d91b9ad470..22bc26d3c993 100644 --- a/cmd/image-builder/config_test.go +++ b/cmd/image-builder/config_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/kyma-project/test-infra/pkg/tags" + "go.uber.org/zap" ) func Test_ParseConfig(t *testing.T) { @@ -385,10 +386,10 @@ func TestLoadGitStateConfig(t *testing.T) { ciSystem: Jenkins, }, env: map[string]string{ - "BRANCH_NAME": "refs/heads/main", - "JENKINS_HOME": "/some/absolute/path", - "GIT_URL": "github.com/kyma-project/test-infra", - "GIT_COMMIT": "1234", + "CHANGE_BRANCH": "refs/heads/main", + "JENKINS_HOME": "/some/absolute/path", + "GIT_URL": "github.com/kyma-project/test-infra.git", + "GIT_COMMIT": "1234", }, gitState: GitStateConfig{ RepositoryName: "test-infra", @@ -403,10 +404,10 @@ func TestLoadGitStateConfig(t *testing.T) { ciSystem: Jenkins, }, env: map[string]string{ - "BRANCH_NAME": "refs/heads/main", + "CHANGE_BRANCH": "refs/heads/main", "JENKINS_HOME": "/some/absolute/path", "CHANGE_ID": "14", - "GIT_URL": "github.com/kyma-project/test-infra", + "GIT_URL": "github.com/kyma-project/test-infra.git", "GIT_COMMIT": "1234", "CHANGE_HEAD_SHA": "4321", // Must be explicitly set when calling docker run }, @@ -415,6 +416,7 @@ func TestLoadGitStateConfig(t *testing.T) { RepositoryOwner: "kyma-project", JobType: "presubmit", BaseCommitSHA: "1234", + BaseCommitRef: "refs/heads/main", PullRequestNumber: 14, PullHeadCommitSHA: "4321", isPullRequest: true, @@ -429,8 +431,15 @@ func TestLoadGitStateConfig(t *testing.T) { t.Setenv(key, value) } + // Setup logger + zapLogger, err := zap.NewDevelopment() + if err != nil { + t.Errorf("Failed to initialize logger: %s", err) + } + logger := zapLogger.Sugar() + // Load git state - state, err := LoadGitStateConfig(c.options.ciSystem) + state, err := LoadGitStateConfig(logger, c.options.ciSystem) if err != nil && !c.expectError { t.Errorf("unexpected error occured %s", err) } @@ -511,4 +520,4 @@ func Test_determineCISystem(t *testing.T) { } }) } -} +} \ No newline at end of file diff --git a/go.mod b/go.mod index e4e02969108c..28ba502ffe12 100644 --- a/go.mod +++ b/go.mod @@ -222,4 +222,4 @@ require ( sigs.k8s.io/controller-runtime v0.15.0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect -) \ No newline at end of file +)