From cbda2d1dfeb9d78e12f3dd7391e4e9da25379977 Mon Sep 17 00:00:00 2001 From: chainchad <96362174+chainchad@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:05:03 -0500 Subject: [PATCH] Move validation of git input into config --- .../internal/updater/config.go | 20 +++++---- .../internal/updater/config_test.go | 44 ++++++++++++++++++- .../internal/updater/updater.go | 16 ------- .../internal/updater/updater_test.go | 31 ------------- 4 files changed, 55 insertions(+), 56 deletions(-) diff --git a/tools/gomod-local-update/internal/updater/config.go b/tools/gomod-local-update/internal/updater/config.go index ed02961a7e3..444193b8086 100644 --- a/tools/gomod-local-update/internal/updater/config.go +++ b/tools/gomod-local-update/internal/updater/config.go @@ -45,20 +45,24 @@ func (c *Config) Validate() error { return nil } + if c.OrgName == "" { + return fmt.Errorf("%w: org name must be provided", ErrInvalidConfig) + } + if c.RepoName == "" { + return fmt.Errorf("%w: repo name must be provided", ErrInvalidConfig) + } if c.RepoRemote == "" { - return fmt.Errorf("%w: repo remote cannot be empty", ErrInvalidConfig) + return fmt.Errorf("%w: repo remote must be provided", ErrInvalidConfig) } - if c.BranchTrunk == "" { - return fmt.Errorf("%w: branch trunk cannot be empty", ErrInvalidConfig) + return fmt.Errorf("%w: trunk branch must be provided", ErrInvalidConfig) } - if c.OrgName == "" { - return fmt.Errorf("%w: organization name cannot be empty", ErrInvalidConfig) + if !gitRemoteRE.MatchString(c.RepoRemote) { + return fmt.Errorf("%w: git remote '%s' contains invalid characters", ErrInvalidConfig, c.RepoRemote) } - - if c.RepoName == "" { - return fmt.Errorf("%w: repository name cannot be empty", ErrInvalidConfig) + if !gitBranchRE.MatchString(c.BranchTrunk) { + return fmt.Errorf("%w: git branch '%s' contains invalid characters", ErrInvalidConfig, c.BranchTrunk) } return nil diff --git a/tools/gomod-local-update/internal/updater/config_test.go b/tools/gomod-local-update/internal/updater/config_test.go index 79fe72a4b9b..21b867aea8d 100644 --- a/tools/gomod-local-update/internal/updater/config_test.go +++ b/tools/gomod-local-update/internal/updater/config_test.go @@ -1,6 +1,9 @@ package updater -import "testing" +import ( + "errors" + "testing" +) func TestConfig_Validate(t *testing.T) { tests := []struct { @@ -61,6 +64,26 @@ func TestConfig_Validate(t *testing.T) { }, wantErr: true, }, + { + name: "invalid remote characters", + config: &Config{ + OrgName: "test", + RepoName: "test", + RepoRemote: "origin!@#", + BranchTrunk: "main", + }, + wantErr: true, + }, + { + name: "invalid branch characters", + config: &Config{ + OrgName: "test", + RepoName: "test", + RepoRemote: "origin", + BranchTrunk: "main!@#", + }, + wantErr: true, + }, } for _, tt := range tests { @@ -73,6 +96,25 @@ func TestConfig_Validate(t *testing.T) { } } +func TestConfig_ValidateErrorType(t *testing.T) { + cfg := &Config{ + RepoRemote: "invalid*remote", + BranchTrunk: "develop", + OrgName: "test", + RepoName: "test", + } + + err := cfg.Validate() + if err == nil { + t.Error("expected error due to invalid repo remote, got nil") + return + } + + if !errors.Is(err, ErrInvalidConfig) { + t.Errorf("expected error to be ErrInvalidConfig, got: %v", err) + } +} + func TestParseFlags(t *testing.T) { tests := []struct { name string diff --git a/tools/gomod-local-update/internal/updater/updater.go b/tools/gomod-local-update/internal/updater/updater.go index 268949c31a5..d44e144192f 100644 --- a/tools/gomod-local-update/internal/updater/updater.go +++ b/tools/gomod-local-update/internal/updater/updater.go @@ -65,18 +65,6 @@ func New(config *Config, system SystemOperator) *Updater { } } -// validateGitInput checks if the remote and branch are in the correct format -func (u *Updater) validateGitInput(remote, branch string) error { - if !gitRemoteRE.MatchString(remote) { - return fmt.Errorf("%w: git remote '%s' contains invalid characters", ErrInvalidConfig, remote) - } - - if !gitBranchRE.MatchString(branch) { - return fmt.Errorf("%w: git branch '%s' contains invalid characters", ErrInvalidConfig, branch) - } - return nil -} - // validateSHA checks if the SHA consists of exactly 40 hexadecimal digits func (u *Updater) validateSHA(sha string) error { if !gitShaRE.MatchString(sha) { @@ -87,10 +75,6 @@ func (u *Updater) validateSHA(sha string) error { // getGitInfo retrieves the latest commit SHA and timestamp from a Git repository func (u *Updater) getGitInfo(remote, branch string) (string, time.Time, error) { - if err := u.validateGitInput(remote, branch); err != nil { - return "", time.Time{}, err - } - ctx, cancel := context.WithTimeout(context.Background(), gitTimeout) defer cancel() diff --git a/tools/gomod-local-update/internal/updater/updater_test.go b/tools/gomod-local-update/internal/updater/updater_test.go index b7c848f2269..da1af2f97e0 100644 --- a/tools/gomod-local-update/internal/updater/updater_test.go +++ b/tools/gomod-local-update/internal/updater/updater_test.go @@ -2,7 +2,6 @@ package updater import ( "context" - "errors" "fmt" "os" "strings" @@ -286,36 +285,6 @@ replace github.com/smartcontractkit/chainlink/v3 => ../ } } -func TestUpdater_Run_InvalidGitInput(t *testing.T) { - cfg := &Config{ - RepoRemote: "invalid*remote", - BranchTrunk: "develop", - OrgName: "smartcontractkit", - RepoName: "chainlink", - } - sysOp := newMockSystemOperator() - sysOp.files["go.mod"] = []byte(`module test -require github.com/smartcontractkit/chainlink/v2 v2.0.0 -replace github.com/smartcontractkit/chainlink/v2 => ../ -`) - - u := New(cfg, sysOp) - u.git = &mockGitExecutor{ - sha: "ac7a7395feed" + strings.Repeat("0", 28), - time: time.Now(), - } - err := u.Run() - if err == nil { - t.Errorf("expected error due to invalid repo remote, got nil") - return - } - - // Use errors.Is instead of errors.As since ErrInvalidConfig is a sentinel error - if !errors.Is(err, ErrInvalidConfig) { - t.Errorf("expected error to be ErrInvalidConfig, got: %v", err) - } -} - func TestUpdater_FindLocalReplaceModules(t *testing.T) { sysOp := newMockSystemOperator() sysOp.files["go.mod"] = []byte(`