From 533e81e574b54f191092561705b54a6d8a5c8504 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Tue, 12 Dec 2023 19:54:04 +0100 Subject: [PATCH 01/28] docs: fixed docstring Signed-off-by: Bruno Bressi --- pkg/config/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/http.go b/pkg/config/http.go index ccd4f2e3..2f694fe1 100644 --- a/pkg/config/http.go +++ b/pkg/config/http.go @@ -42,7 +42,7 @@ func NewHttpLoader(cfg *Config, cCfgChecks chan<- map[string]any) *HttpLoader { } } -// GetRuntimeConfig gets the runtime configuration +// Run gets the runtime configuration // from the http remote endpoint. // The config is will be loaded periodically defined by the // loader interval configuration. A failed request will be retried defined From da1a9837cf9d1bb9f6cbe4217f6685c3b96396fc Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Tue, 12 Dec 2023 20:47:13 +0100 Subject: [PATCH 02/28] feat: added TargetManager to handle global targets Signed-off-by: Bruno Bressi --- pkg/config/config.go | 11 ++++++ pkg/sparrow/run.go | 85 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 50f9fb15..238423ac 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -24,10 +24,21 @@ import ( "github.com/caas-team/sparrow/internal/helper" ) +// TargetConfig is the configuration for the target +// reconciliation process +type TargetConfig struct { + // the interval in seconds for the target reconciliation process + Interval time.Duration + // the amount of time in seconds a target can be + // unhealthy before it is removed from the global target list + UnhealthyThreshold time.Duration +} + type Config struct { Checks map[string]any Loader LoaderConfig Api ApiConfig + Target TargetConfig } // ApiConfig is the configuration for the data API diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 0e43f57d..0cad1126 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "net/http" + "time" "github.com/caas-team/sparrow/internal/logger" "github.com/caas-team/sparrow/pkg/api" @@ -31,9 +32,27 @@ import ( "github.com/go-chi/chi/v5" ) +// globalTarget represents a globalTarget that can be checked +type globalTarget struct { + url string + lastSeen time.Time +} + +// TargetManager handles the management of globalTargets for +// a Sparrow instance +type TargetManager interface { + Reconcile(ctx context.Context) + GetTargets() +} + +// gitlabTargetManager implements TargetManager +type gitlabTargetManager struct { + targets []globalTarget + client *http.Client + cfg *config.TargetConfig +} + type Sparrow struct { - // TODO refactor this struct to be less convoluted - // split up responsibilities more clearly db db.DB // the existing checks checks map[string]checks.Check @@ -45,6 +64,7 @@ type Sparrow struct { cfg *config.Config loader config.Loader cCfgChecks chan map[string]any + targets TargetManager routingTree *api.RoutingTree router chi.Router @@ -74,8 +94,9 @@ func (s *Sparrow) Run(ctx context.Context) error { ctx, cancel := logger.NewContextWithLogger(ctx, "sparrow") defer cancel() - // Start the runtime configuration loader go s.loader.Run(ctx) + go s.targets.Reconcile(ctx) + // Start the api go func() { err := s.api(ctx) if err != nil { @@ -172,6 +193,64 @@ func (s *Sparrow) ReconcileChecks(ctx context.Context) { } } +// Reconcile reconciles the targets of the gitlabTargetManager. +// The global gitlabTargetManager are parsed from a remote endpoint. +// +// The global gitlabTargetManager are evaluated for healthiness and +// unhealthy gitlabTargetManager are removed. +func (t *gitlabTargetManager) Reconcile(ctx context.Context) { + log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") + log.Debug("Starting global gitlabTargetManager reconciler") + + for { + // start a timer + timer := time.NewTimer(t.cfg.Interval) + defer timer.Stop() + + select { + case <-ctx.Done(): + if err := ctx.Err(); err != nil { + log.Error("Context canceled", "error", err) + return + } + case <-timer.C: + log.Debug("Getting global gitlabTargetManager") + targets, err := t.getTargets(ctx) + t.updateTargets(targets) + if err != nil { + log.Error("Failed to get global gitlabTargetManager", "error", err) + continue + } + } + } +} + +// GetTargets returns the current targets of the gitlabTargetManager +func (t *gitlabTargetManager) GetTargets() []globalTarget { + return t.targets +} + +// getGlobalTargets gets the global gitlabTargetManager from the configured gitlab repository +func (t *gitlabTargetManager) getTargets(ctx context.Context) ([]globalTarget, error) { + log := logger.FromContext(ctx).With("name", "getGlobalTargets") + log.Debug("Getting global gitlabTargetManager") + var res []globalTarget + return res, nil +} + +// updateTargets sets the global gitlabTargetManager +func (t *gitlabTargetManager) updateTargets(targets []globalTarget) { + var healthyTargets []globalTarget + for _, target := range targets { + if time.Now().Add(-t.cfg.UnhealthyThreshold).After(target.lastSeen) { + continue + } + healthyTargets = append(healthyTargets, target) + } + + t.targets = healthyTargets +} + // This is a fan in for the checks. // // It allows augmenting the results with the check name which is needed by the db From d2899c1a8d4f828c74495c72f0ab0a505115e4cd Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Tue, 12 Dec 2023 23:03:46 +0100 Subject: [PATCH 03/28] refactor: added gitlab interface to handle interaction with the API Signed-off-by: Bruno Bressi --- pkg/config/config.go | 11 --- pkg/sparrow/run.go | 82 +----------------- pkg/sparrow/targets/gitlab.go | 147 +++++++++++++++++++++++++++++++++ pkg/sparrow/targets/targets.go | 20 +++++ 4 files changed, 170 insertions(+), 90 deletions(-) create mode 100644 pkg/sparrow/targets/gitlab.go create mode 100644 pkg/sparrow/targets/targets.go diff --git a/pkg/config/config.go b/pkg/config/config.go index 238423ac..50f9fb15 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -24,21 +24,10 @@ import ( "github.com/caas-team/sparrow/internal/helper" ) -// TargetConfig is the configuration for the target -// reconciliation process -type TargetConfig struct { - // the interval in seconds for the target reconciliation process - Interval time.Duration - // the amount of time in seconds a target can be - // unhealthy before it is removed from the global target list - UnhealthyThreshold time.Duration -} - type Config struct { Checks map[string]any Loader LoaderConfig Api ApiConfig - Target TargetConfig } // ApiConfig is the configuration for the data API diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 0cad1126..41d888d9 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -29,29 +29,10 @@ import ( "github.com/caas-team/sparrow/pkg/checks" "github.com/caas-team/sparrow/pkg/config" "github.com/caas-team/sparrow/pkg/db" + targets "github.com/caas-team/sparrow/pkg/sparrow/targets" "github.com/go-chi/chi/v5" ) -// globalTarget represents a globalTarget that can be checked -type globalTarget struct { - url string - lastSeen time.Time -} - -// TargetManager handles the management of globalTargets for -// a Sparrow instance -type TargetManager interface { - Reconcile(ctx context.Context) - GetTargets() -} - -// gitlabTargetManager implements TargetManager -type gitlabTargetManager struct { - targets []globalTarget - client *http.Client - cfg *config.TargetConfig -} - type Sparrow struct { db db.DB // the existing checks @@ -64,7 +45,7 @@ type Sparrow struct { cfg *config.Config loader config.Loader cCfgChecks chan map[string]any - targets TargetManager + targets targets.TargetManager routingTree *api.RoutingTree router chi.Router @@ -82,6 +63,7 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), + targets: targets.NewGitlabManager(targets.NewGitlabClient("targetsRepo", "gitlabToken"), 5*time.Minute, 15*time.Minute), } sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) @@ -193,64 +175,6 @@ func (s *Sparrow) ReconcileChecks(ctx context.Context) { } } -// Reconcile reconciles the targets of the gitlabTargetManager. -// The global gitlabTargetManager are parsed from a remote endpoint. -// -// The global gitlabTargetManager are evaluated for healthiness and -// unhealthy gitlabTargetManager are removed. -func (t *gitlabTargetManager) Reconcile(ctx context.Context) { - log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") - log.Debug("Starting global gitlabTargetManager reconciler") - - for { - // start a timer - timer := time.NewTimer(t.cfg.Interval) - defer timer.Stop() - - select { - case <-ctx.Done(): - if err := ctx.Err(); err != nil { - log.Error("Context canceled", "error", err) - return - } - case <-timer.C: - log.Debug("Getting global gitlabTargetManager") - targets, err := t.getTargets(ctx) - t.updateTargets(targets) - if err != nil { - log.Error("Failed to get global gitlabTargetManager", "error", err) - continue - } - } - } -} - -// GetTargets returns the current targets of the gitlabTargetManager -func (t *gitlabTargetManager) GetTargets() []globalTarget { - return t.targets -} - -// getGlobalTargets gets the global gitlabTargetManager from the configured gitlab repository -func (t *gitlabTargetManager) getTargets(ctx context.Context) ([]globalTarget, error) { - log := logger.FromContext(ctx).With("name", "getGlobalTargets") - log.Debug("Getting global gitlabTargetManager") - var res []globalTarget - return res, nil -} - -// updateTargets sets the global gitlabTargetManager -func (t *gitlabTargetManager) updateTargets(targets []globalTarget) { - var healthyTargets []globalTarget - for _, target := range targets { - if time.Now().Add(-t.cfg.UnhealthyThreshold).After(target.lastSeen) { - continue - } - healthyTargets = append(healthyTargets, target) - } - - t.targets = healthyTargets -} - // This is a fan in for the checks. // // It allows augmenting the results with the check name which is needed by the db diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go new file mode 100644 index 00000000..47347a41 --- /dev/null +++ b/pkg/sparrow/targets/gitlab.go @@ -0,0 +1,147 @@ +package targets + +import ( + "context" + "net/http" + "time" + + "github.com/caas-team/sparrow/internal/logger" +) + +var ( + _ Gitlab = &gitlab{} + _ TargetManager = &gitlabTargetManager{} +) + +// Gitlab handles interaction with a gitlab repository containing +// the global targets for the Sparrow instance +type Gitlab interface { + ReadGlobalTargets(ctx context.Context) ([]globalTarget, error) + RegisterSelf(ctx context.Context) error +} + +// gitlabTargetManager implements TargetManager +type gitlabTargetManager struct { + targets []globalTarget + gitlab Gitlab + // the interval for the target reconciliation process + checkInterval time.Duration + // the amount of time a target can be + // unhealthy before it is removed from the global target list + unhealthyThreshold time.Duration +} + +// NewGitlabManager creates a new gitlabTargetManager +func NewGitlabManager(g Gitlab, checkInterval, unhealthyThreshold time.Duration) TargetManager { + return &gitlabTargetManager{ + targets: []globalTarget{}, + gitlab: g, + checkInterval: checkInterval, + unhealthyThreshold: unhealthyThreshold, + } +} + +// gitlab implements Gitlab +type gitlab struct { + url string + token string + client *http.Client +} + +func NewGitlabClient(url, token string) Gitlab { + return &gitlab{ + url: url, + token: token, + client: &http.Client{}, + } +} + +func (t *gitlabTargetManager) Register(ctx context.Context) { + log := logger.FromContext(ctx).With("name", "RegisterGlobalTargets") + log.Debug("Registering global gitlabTargetManager") + + err := t.gitlab.RegisterSelf(ctx) + if err != nil { + log.Error("Failed to register global gitlabTargetManager", "error", err) + } +} + +// Reconcile reconciles the targets of the gitlabTargetManager. +// The global gitlabTargetManager are parsed from a remote endpoint. +// +// The global gitlabTargetManager are evaluated for healthiness and +// unhealthy gitlabTargetManager are removed. +func (t *gitlabTargetManager) Reconcile(ctx context.Context) { + log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") + log.Debug("Starting global gitlabTargetManager reconciler") + + for { + // start a timer + timer := time.NewTimer(t.checkInterval) + defer timer.Stop() + + select { + case <-ctx.Done(): + if err := ctx.Err(); err != nil { + log.Error("Context canceled", "error", err) + return + } + case <-timer.C: + log.Debug("Getting global gitlabTargetManager") + err := t.updateTargets(ctx) + if err != nil { + log.Error("Failed to get global gitlabTargetManager", "error", err) + continue + } + } + } +} + +// GetTargets returns the current targets of the gitlabTargetManager +func (t *gitlabTargetManager) GetTargets() []globalTarget { + return t.targets +} + +// updateTargets sets the global gitlabTargetManager +func (t *gitlabTargetManager) updateTargets(ctx context.Context) error { + log := logger.FromContext(ctx).With("name", "updateGlobalTargets") + var healthyTargets []globalTarget + + targets, err := t.gitlab.ReadGlobalTargets(ctx) + if err != nil { + log.Error("Failed to update global targets", "error", err) + return err + } + + for _, target := range targets { + if time.Now().Add(-t.unhealthyThreshold).After(target.lastSeen) { + continue + } + healthyTargets = append(healthyTargets, target) + } + + t.targets = healthyTargets + log.Debug("Updated global targets", "targets", len(t.targets)) + return nil +} + +// ReadGlobalTargets fetches the global gitlabTargetManager from the configured gitlab repository +func (g *gitlab) ReadGlobalTargets(ctx context.Context) ([]globalTarget, error) { + log := logger.FromContext(ctx).With("name", "ReadGlobalTargets") + log.Debug("Fetching global gitlabTargetManager") + + // TODO: pull file list from repo and marshal into []globalTarget + + return nil, nil +} + +// RegisterSelf commits the current instance to the configured gitlab repository +// as a global target for other sparrow instances to discover +func (g *gitlab) RegisterSelf(ctx context.Context) error { + log := logger.FromContext(ctx).With("name", "RegisterSelf") + log.Debug("Registering sparrow instance to gitlab") + + // TODO: update & commit self as target to gitlab + + return nil +} diff --git a/pkg/sparrow/targets/targets.go b/pkg/sparrow/targets/targets.go new file mode 100644 index 00000000..23e2d8f5 --- /dev/null +++ b/pkg/sparrow/targets/targets.go @@ -0,0 +1,20 @@ +package targets + +import ( + "context" + "time" +) + +// globalTarget represents a globalTarget that can be checked +type globalTarget struct { + url string + lastSeen time.Time +} + +// TargetManager handles the management of globalTargets for +// a Sparrow instance +type TargetManager interface { + Reconcile(ctx context.Context) + Register(ctx context.Context) + GetTargets() []globalTarget +} From 8327b34bd15ce2fdf43a93029fc314097414aaac Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Wed, 13 Dec 2023 17:55:32 +0100 Subject: [PATCH 04/28] chore: feat all gitlab functions Signed-off-by: Bruno Bressi --- pkg/sparrow/run.go | 2 +- pkg/sparrow/targets/gitlab.go | 308 +++++++++++++++++++++--- pkg/sparrow/targets/gitlab_test.go | 367 +++++++++++++++++++++++++++++ pkg/sparrow/targets/targets.go | 5 +- 4 files changed, 641 insertions(+), 41 deletions(-) create mode 100644 pkg/sparrow/targets/gitlab_test.go diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 41d888d9..92199f62 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -63,7 +63,7 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), - targets: targets.NewGitlabManager(targets.NewGitlabClient("targetsRepo", "gitlabToken"), 5*time.Minute, 15*time.Minute), + targets: targets.NewGitlabManager(targets.NewGitlab("targetsRepo", "gitlabToken"), 5*time.Minute, 15*time.Minute), } sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index 47347a41..3e660da3 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -1,8 +1,12 @@ package targets import ( + "bytes" "context" + "encoding/json" + "fmt" "net/http" + "net/url" "time" "github.com/caas-team/sparrow/internal/logger" @@ -16,19 +20,37 @@ var ( // Gitlab handles interaction with a gitlab repository containing // the global targets for the Sparrow instance type Gitlab interface { - ReadGlobalTargets(ctx context.Context) ([]globalTarget, error) - RegisterSelf(ctx context.Context) error + FetchFiles(ctx context.Context) ([]globalTarget, error) + PutFile(ctx context.Context, file GitlabFile) error + PostFile(ctx context.Context, file GitlabFile) error +} + +// gitlab implements Gitlab +type gitlab struct { + // the base URL of the gitlab instance + baseUrl string + // the ID of the project containing the global targets + projectID int + // the token used to authenticate with the gitlab instance + token string + client *http.Client } // gitlabTargetManager implements TargetManager type gitlabTargetManager struct { targets []globalTarget gitlab Gitlab + // the DNS name used for self-registration + name string // the interval for the target reconciliation process checkInterval time.Duration // the amount of time a target can be // unhealthy before it is removed from the global target list unhealthyThreshold time.Duration + // how often the instance should register itself as a global target + registrationInterval time.Duration + // whether the instance has already registered itself as a global target + registered bool } // NewGitlabManager creates a new gitlabTargetManager @@ -41,58 +63,84 @@ func NewGitlabManager(g Gitlab, checkInterval, unhealthyThreshold time.Duration) } } -// gitlab implements Gitlab -type gitlab struct { - url string - token string - client *http.Client +// file represents a file in a gitlab repository +type file struct { + Name string `json:"name"` } -func NewGitlabClient(url, token string) Gitlab { +func NewGitlab(url, token string) Gitlab { return &gitlab{ - url: url, - token: token, - client: &http.Client{}, + baseUrl: url, + token: token, + client: &http.Client{}, } } -func (t *gitlabTargetManager) Register(ctx context.Context) { - log := logger.FromContext(ctx).With("name", "RegisterGlobalTargets") - log.Debug("Registering global gitlabTargetManager") +// updateRegistration registers the current instance as a global target +func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { + log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered) + log.Debug("Updating registration") + + f := GitlabFile{ + Branch: "main", + AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), + AuthorName: t.name, + Content: globalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, + } + + if t.registered { + f.CommitMessage = "Updated registration" + err := t.gitlab.PutFile(ctx, f) + if err != nil { + log.Error("Failed to update registration", "error", err) + return err + } + log.Debug("Successfully updated registration") + return nil + } - err := t.gitlab.RegisterSelf(ctx) + f.CommitMessage = "Initial registration" + err := t.gitlab.PostFile(ctx, f) if err != nil { log.Error("Failed to register global gitlabTargetManager", "error", err) } + + log.Debug("Successfully registered") + t.registered = true + return nil } // Reconcile reconciles the targets of the gitlabTargetManager. -// The global gitlabTargetManager are parsed from a remote endpoint. +// The global targets are parsed from a remote endpoint. // -// The global gitlabTargetManager are evaluated for healthiness and +// The global targets are evaluated for healthiness and // unhealthy gitlabTargetManager are removed. func (t *gitlabTargetManager) Reconcile(ctx context.Context) { log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") log.Debug("Starting global gitlabTargetManager reconciler") for { - // start a timer - timer := time.NewTimer(t.checkInterval) - defer timer.Stop() - select { case <-ctx.Done(): if err := ctx.Err(); err != nil { log.Error("Context canceled", "error", err) return } - case <-timer.C: + // check if this blocks when context is canceled + case <-time.After(t.checkInterval): log.Debug("Getting global gitlabTargetManager") - err := t.updateTargets(ctx) + err := t.refreshTargets(ctx) if err != nil { log.Error("Failed to get global gitlabTargetManager", "error", err) continue } + case <-time.After(t.registrationInterval): + log.Debug("Registering global gitlabTargetManager") + err := t.updateRegistration(ctx) + if err != nil { + log.Error("Failed to register global gitlabTargetManager", "error", err) + continue + } } } } @@ -102,19 +150,21 @@ func (t *gitlabTargetManager) GetTargets() []globalTarget { return t.targets } -// updateTargets sets the global gitlabTargetManager -func (t *gitlabTargetManager) updateTargets(ctx context.Context) error { +// refreshTargets updates the targets of the gitlabTargetManager +// with the latest available healthy targets +func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { log := logger.FromContext(ctx).With("name", "updateGlobalTargets") var healthyTargets []globalTarget - targets, err := t.gitlab.ReadGlobalTargets(ctx) + targets, err := t.gitlab.FetchFiles(ctx) if err != nil { log.Error("Failed to update global targets", "error", err) return err } + // filter unhealthy targets - this may be removed in the future for _, target := range targets { - if time.Now().Add(-t.unhealthyThreshold).After(target.lastSeen) { + if time.Now().Add(-t.unhealthyThreshold).After(target.LastSeen) { continue } healthyTargets = append(healthyTargets, target) @@ -125,23 +175,207 @@ func (t *gitlabTargetManager) updateTargets(ctx context.Context) error { return nil } -// ReadGlobalTargets fetches the global gitlabTargetManager from the configured gitlab repository -func (g *gitlab) ReadGlobalTargets(ctx context.Context) ([]globalTarget, error) { - log := logger.FromContext(ctx).With("name", "ReadGlobalTargets") - log.Debug("Fetching global gitlabTargetManager") +// FetchFiles fetches the files from the global targets repository from the configured gitlab repository +func (g *gitlab) FetchFiles(ctx context.Context) ([]globalTarget, error) { + log := logger.FromContext(ctx).With("name", "FetchFiles") + fl, err := g.fetchFileList(ctx) + if err != nil { + log.Error("Failed to fetch files", "error", err) + return nil, err + } + + result, err := g.fetchFiles(ctx, fl) + if err != nil { + log.Error("Failed to fetch files", "error", err) + return nil, err + } + log.Info("Successfully fetched all target files", "files", len(result)) + return result, nil +} + +// fetchFiles fetches the files from the global targets repository from the configured gitlab repository +func (g *gitlab) fetchFiles(ctx context.Context, fl []string) ([]globalTarget, error) { + var result []globalTarget + log := logger.FromContext(ctx).With("name", "fetchFiles") + log.Debug("Fetching global files") + for _, f := range fl { + // URL encode the name + n := url.PathEscape(f) + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s/raw?ref=main", g.baseUrl, g.projectID, n), + http.NoBody, + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return nil, err + } + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + res, err := g.client.Do(req) + if err != nil { + log.Error("Failed to fetch file", "file", f, "error", err) + return nil, err + } + if res.StatusCode != http.StatusOK { + log.Error("Failed to fetch file", "status", res.Status) + return nil, fmt.Errorf("request failed, status is %s", res.Status) + } + + defer res.Body.Close() + var gt globalTarget + err = json.NewDecoder(res.Body).Decode(>) + if err != nil { + log.Error("Failed to decode file after fetching", "file", f, "error", err) + return nil, err + } + + log.Debug("Successfully fetched file", "file", f) + result = append(result, gt) + } + return result, nil +} + +// fetchFileList fetches the files from the global targets repository from the configured gitlab repository +func (g *gitlab) fetchFileList(ctx context.Context) ([]string, error) { + log := logger.FromContext(ctx).With("name", "fetchFileList") + log.Debug("Fetching global files") + type file struct { + Name string `json:"name"` + } + + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/tree?ref=main", g.baseUrl, g.projectID), + http.NoBody, + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return nil, err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + res, err := g.client.Do(req) + if err != nil { + log.Error("Failed to fetch file list", "error", err) + return nil, err + } + if res.StatusCode != http.StatusOK { + log.Error("Failed to fetch file list", "status", res.Status) + return nil, fmt.Errorf("request failed, status is %s", res.Status) + } + + defer res.Body.Close() + var fl []file + err = json.NewDecoder(res.Body).Decode(&fl) + if err != nil { + log.Error("Failed to decode file list", "error", err) + return nil, err + } + + var result []string + for _, f := range fl { + result = append(result, f.Name) + } + + log.Debug("Successfully fetched file list", "files", len(result)) + return result, nil +} + +type GitlabFile struct { + Branch string `json:"branch"` + AuthorEmail string `json:"author_email"` + AuthorName string `json:"author_name"` + Content globalTarget `json:"content"` + CommitMessage string `json:"commit_message"` + fileName string +} + +// Bytes returns the bytes of the GitlabFile +func (g GitlabFile) Bytes() ([]byte, error) { + b, err := json.Marshal(g) + return b, err +} + +// PutFile commits the current instance to the configured gitlab repository +// as a global target for other sparrow instances to discover +func (g *gitlab) PutFile(ctx context.Context, body GitlabFile) error { + log := logger.FromContext(ctx).With("name", "AddRegistration") + log.Debug("Registering sparrow instance to gitlab") + + // chose method based on whether the registration has already happened + n := url.PathEscape(body.Content.Url) + b, err := body.Bytes() + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), + bytes.NewBuffer(b), + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") - // TODO: pull file list from repo and marshal into []globalTarget + resp, err := g.client.Do(req) + if err != nil { + log.Error("Failed to push registration file", "error", err) + return err + } - return nil, nil + if resp.StatusCode != http.StatusAccepted { + log.Error("Failed to push registration file", "status", resp.Status) + return fmt.Errorf("request failed, status is %s", resp.Status) + } + + return nil } -// RegisterSelf commits the current instance to the configured gitlab repository +// PostFile commits the current instance to the configured gitlab repository // as a global target for other sparrow instances to discover -func (g *gitlab) RegisterSelf(ctx context.Context) error { - log := logger.FromContext(ctx).With("name", "RegisterSelf") +func (g *gitlab) PostFile(ctx context.Context, body GitlabFile) error { + log := logger.FromContext(ctx).With("name", "AddRegistration") log.Debug("Registering sparrow instance to gitlab") - // TODO: update & commit self as target to gitlab + // chose method based on whether the registration has already happened + n := url.PathEscape(body.Content.Url) + b, err := body.Bytes() + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + req, err := http.NewRequestWithContext(ctx, + http.MethodPost, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), + bytes.NewBuffer(b), + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + resp, err := g.client.Do(req) + if err != nil { + log.Error("Failed to push registration file", "error", err) + return err + } + + if resp.StatusCode != http.StatusCreated { + log.Error("Failed to push registration file", "status", resp.Status) + return fmt.Errorf("request failed, status is %s", resp.Status) + } return nil } diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go new file mode 100644 index 00000000..eebe7313 --- /dev/null +++ b/pkg/sparrow/targets/gitlab_test.go @@ -0,0 +1,367 @@ +package targets + +import ( + "context" + "fmt" + "net/http" + "reflect" + "testing" + "time" + + "github.com/jarcoal/httpmock" +) + +func Test_gitlab_fetchFileList(t *testing.T) { + type file struct { + Name string `json:"name"` + } + tests := []struct { + name string + want []string + wantErr bool + mockBody []file + mockCode int + }{ + { + name: "success - 0 targets", + want: nil, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{}, + }, + { + name: "success - 1 target", + want: []string{ + "test", + }, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{ + { + Name: "test", + }, + }, + }, + { + name: "success - 2 targets", + want: []string{ + "test", + "test2", + }, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{ + { + Name: "test", + }, + { + Name: "test2", + }, + }, + }, + { + name: "failure - API error", + want: nil, + wantErr: true, + mockCode: http.StatusInternalServerError, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.mockBody) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", "http://test/api/v4/projects/1/repository/tree?ref=main", resp) + + g := &gitlab{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + got, err := g.fetchFileList(context.Background()) + if (err != nil) != tt.wantErr { + t.Errorf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("FetchFiles() got = %v, want %v", got, tt.want) + } + }) + } +} + +// The filelist and url are the same, so we HTTP responders can +// be created without much hassle +func Test_gitlab_fetchFiles(t *testing.T) { + tests := []struct { + name string + want []globalTarget + fileList []string + wantErr bool + mockCode int + }{ + { + name: "success - 0 targets", + want: nil, + wantErr: false, + mockCode: http.StatusOK, + }, + { + name: "success - 1 target", + want: []globalTarget{ + { + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + fileList: []string{ + "test", + }, + wantErr: false, + mockCode: http.StatusOK, + }, + { + name: "success - 2 targets", + want: []globalTarget{ + { + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + { + Url: "test2", + LastSeen: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), + }, + }, + fileList: []string{ + "test", + "test2", + }, + wantErr: false, + mockCode: http.StatusOK, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &gitlab{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup mock responses + for i, target := range tt.want { + resp, err := httpmock.NewJsonResponder(tt.mockCode, target) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + } + + got, err := g.fetchFiles(context.Background(), tt.fileList) + if (err != nil) != tt.wantErr { + t.Fatalf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("FetchFiles() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_gitlab_fetchFiles_error_cases(t *testing.T) { + type mockResponses struct { + response globalTarget + err bool + } + + tests := []struct { + name string + mockResponses []mockResponses + fileList []string + }{ + { + name: "failure - direct API error", + mockResponses: []mockResponses{ + { + err: true, + }, + }, + fileList: []string{ + "test", + }, + }, + { + name: "failure - API error after one successful request", + mockResponses: []mockResponses{ + { + response: globalTarget{ + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + err: false, + }, + { + response: globalTarget{}, + err: true, + }, + }, + fileList: []string{ + "test", + "test2-will-fail", + }, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &gitlab{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for i, target := range tt.mockResponses { + if target.err { + errResp := httpmock.NewStringResponder(http.StatusInternalServerError, "") + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), errResp) + continue + } + resp, err := httpmock.NewJsonResponder(http.StatusOK, target) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + } + + _, err := g.fetchFiles(context.Background(), tt.fileList) + if err == nil { + t.Fatalf("Expected error but got none.") + } + }) + } +} + +func Test_gitlabTargetManager_refreshTargets(t *testing.T) { + now := time.Now() + tooOld := now.Add(-time.Hour * 2) + + tests := []struct { + name string + mockTargets []globalTarget + expectedHealthy []globalTarget + wantErr bool + }{ + { + name: "success with 0 targets", + mockTargets: []globalTarget{}, + expectedHealthy: []globalTarget{}, + }, + { + name: "success with 1 healthy target", + mockTargets: []globalTarget{ + { + Url: "test", + LastSeen: now, + }, + }, + expectedHealthy: []globalTarget{ + { + Url: "test", + LastSeen: now, + }, + }, + }, + { + name: "success with 1 unhealthy target", + mockTargets: []globalTarget{ + { + Url: "test", + LastSeen: tooOld, + }, + }, + }, + { + name: "success with 1 healthy and 1 unhealthy targets", + mockTargets: []globalTarget{ + { + Url: "test", + LastSeen: now, + }, + { + Url: "test2", + LastSeen: tooOld, + }, + }, + expectedHealthy: []globalTarget{ + { + Url: "test", + LastSeen: now, + }, + }, + }, + { + name: "failure getting targets", + mockTargets: nil, + expectedHealthy: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gtm := &gitlabTargetManager{ + targets: nil, + gitlab: newMockGitlab(tt.mockTargets, tt.wantErr), + name: "test", + unhealthyThreshold: time.Hour, + } + if err := gtm.refreshTargets(context.Background()); (err != nil) != tt.wantErr { + t.Fatalf("refreshTargets() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +type mockGitlab struct { + targets []globalTarget + err error +} + +func (m mockGitlab) PutFile(ctx context.Context, file GitlabFile) error { + panic("implement me") +} + +func (m mockGitlab) PostFile(ctx context.Context, f GitlabFile) error { + panic("implement me") +} + +func (m mockGitlab) FetchFiles(ctx context.Context) ([]globalTarget, error) { + return m.targets, m.err +} + +func (m mockGitlab) FetchFileList(ctx context.Context) ([]string, error) { + panic("implement me") +} + +func newMockGitlab(targets []globalTarget, err bool) Gitlab { + var e error + if err { + e = fmt.Errorf("error") + } + return &mockGitlab{ + targets: targets, + err: e, + } +} diff --git a/pkg/sparrow/targets/targets.go b/pkg/sparrow/targets/targets.go index 23e2d8f5..5f1eb2ad 100644 --- a/pkg/sparrow/targets/targets.go +++ b/pkg/sparrow/targets/targets.go @@ -7,14 +7,13 @@ import ( // globalTarget represents a globalTarget that can be checked type globalTarget struct { - url string - lastSeen time.Time + Url string `json:"url"` + LastSeen time.Time `json:"lastSeen"` } // TargetManager handles the management of globalTargets for // a Sparrow instance type TargetManager interface { Reconcile(ctx context.Context) - Register(ctx context.Context) GetTargets() []globalTarget } From e19ccd45997539bb39b218c2a4fe428db33e1dd3 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Wed, 13 Dec 2023 22:47:17 +0100 Subject: [PATCH 05/28] tests: test post & put methods Signed-off-by: Bruno Bressi --- pkg/checks/checks.go | 4 + pkg/sparrow/gitlab/gitlab.go | 250 ++++++++++++++++ pkg/sparrow/gitlab/gitlab_test.go | 403 ++++++++++++++++++++++++++ pkg/sparrow/gitlab/test/mockclient.go | 38 +++ pkg/sparrow/run.go | 12 +- pkg/sparrow/targets/gitlab.go | 264 +---------------- pkg/sparrow/targets/gitlab_test.go | 301 +------------------ pkg/sparrow/targets/targetmanager.go | 14 + pkg/sparrow/targets/targets.go | 19 -- 9 files changed, 742 insertions(+), 563 deletions(-) create mode 100644 pkg/sparrow/gitlab/gitlab.go create mode 100644 pkg/sparrow/gitlab/gitlab_test.go create mode 100644 pkg/sparrow/gitlab/test/mockclient.go create mode 100644 pkg/sparrow/targets/targetmanager.go delete mode 100644 pkg/sparrow/targets/targets.go diff --git a/pkg/checks/checks.go b/pkg/checks/checks.go index d495ab12..b8cbd6b0 100644 --- a/pkg/checks/checks.go +++ b/pkg/checks/checks.go @@ -74,6 +74,10 @@ type Result struct { Err string `json:"error"` } +type GlobalTarget struct { + Url string `json:"url"` + LastSeen time.Time `json:"lastSeen"` +} type ResultDTO struct { Name string Result *Result diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go new file mode 100644 index 00000000..7984693a --- /dev/null +++ b/pkg/sparrow/gitlab/gitlab.go @@ -0,0 +1,250 @@ +package gitlab + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + + "github.com/caas-team/sparrow/internal/logger" + "github.com/caas-team/sparrow/pkg/checks" +) + +// Gitlab handles interaction with a gitlab repository containing +// the global targets for the Sparrow instance +type Gitlab interface { + FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) + PutFile(ctx context.Context, file File) error + PostFile(ctx context.Context, file File) error +} + +// Client implements Gitlab +type Client struct { + // the base URL of the gitlab instance + baseUrl string + // the ID of the project containing the global targets + projectID int + // the token used to authenticate with the gitlab instance + token string + client *http.Client +} + +func New(url, token string, pid int) Gitlab { + return &Client{ + baseUrl: url, + token: token, + projectID: pid, + client: &http.Client{}, + } +} + +// FetchFiles fetches the files from the global targets repository from the configured gitlab repository +func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { + log := logger.FromContext(ctx).With("name", "FetchFiles") + fl, err := g.fetchFileList(ctx) + if err != nil { + log.Error("Failed to fetch files", "error", err) + return nil, err + } + + result, err := g.fetchFiles(ctx, fl) + if err != nil { + log.Error("Failed to fetch files", "error", err) + return nil, err + } + log.Info("Successfully fetched all target files", "files", len(result)) + return result, nil +} + +// fetchFiles fetches the files from the global targets repository from the configured gitlab repository +func (g *Client) fetchFiles(ctx context.Context, fl []string) ([]checks.GlobalTarget, error) { + var result []checks.GlobalTarget + log := logger.FromContext(ctx).With("name", "fetchFiles") + log.Debug("Fetching global files") + for _, f := range fl { + // URL encode the name + n := url.PathEscape(f) + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s/raw?ref=main", g.baseUrl, g.projectID, n), + http.NoBody, + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return nil, err + } + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + res, err := g.client.Do(req) + if err != nil { + log.Error("Failed to fetch file", "file", f, "error", err) + return nil, err + } + if res.StatusCode != http.StatusOK { + log.Error("Failed to fetch file", "status", res.Status) + return nil, fmt.Errorf("request failed, status is %s", res.Status) + } + + defer res.Body.Close() + var gt checks.GlobalTarget + err = json.NewDecoder(res.Body).Decode(>) + if err != nil { + log.Error("Failed to decode file after fetching", "file", f, "error", err) + return nil, err + } + + log.Debug("Successfully fetched file", "file", f) + result = append(result, gt) + } + return result, nil +} + +// fetchFileList fetches the files from the global targets repository from the configured gitlab repository +func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { + log := logger.FromContext(ctx).With("name", "fetchFileList") + log.Debug("Fetching global files") + type file struct { + Name string `json:"name"` + } + + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/tree?ref=main", g.baseUrl, g.projectID), + http.NoBody, + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return nil, err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + res, err := g.client.Do(req) + if err != nil { + log.Error("Failed to fetch file list", "error", err) + return nil, err + } + if res.StatusCode != http.StatusOK { + log.Error("Failed to fetch file list", "status", res.Status) + return nil, fmt.Errorf("request failed, status is %s", res.Status) + } + + defer res.Body.Close() + var fl []file + err = json.NewDecoder(res.Body).Decode(&fl) + if err != nil { + log.Error("Failed to decode file list", "error", err) + return nil, err + } + + var result []string + for _, f := range fl { + result = append(result, f.Name) + } + + log.Debug("Successfully fetched file list", "files", len(result)) + return result, nil +} + +// PutFile commits the current instance to the configured gitlab repository +// as a global target for other sparrow instances to discover +func (g *Client) PutFile(ctx context.Context, body File) error { + log := logger.FromContext(ctx).With("name", "AddRegistration") + log.Debug("Registering sparrow instance to gitlab") + + // chose method based on whether the registration has already happened + n := url.PathEscape(body.fileName) + b, err := body.Bytes() + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + req, err := http.NewRequestWithContext(ctx, + http.MethodPut, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), + bytes.NewBuffer(b), + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + resp, err := g.client.Do(req) + if err != nil { + log.Error("Failed to push registration file", "error", err) + return err + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + log.Error("Failed to push registration file", "status", resp.Status) + return fmt.Errorf("request failed, status is %s", resp.Status) + } + + return nil +} + +// PostFile commits the current instance to the configured gitlab repository +// as a global target for other sparrow instances to discover +func (g *Client) PostFile(ctx context.Context, body File) error { + log := logger.FromContext(ctx).With("name", "AddRegistration") + log.Debug("Registering sparrow instance to gitlab") + + // chose method based on whether the registration has already happened + n := url.PathEscape(body.fileName) + b, err := body.Bytes() + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + req, err := http.NewRequestWithContext(ctx, + http.MethodPost, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), + bytes.NewBuffer(b), + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return err + } + + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") + + resp, err := g.client.Do(req) + if err != nil { + log.Error("Failed to push registration file", "error", err) + return err + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + log.Error("Failed to push registration file", "status", resp.Status) + return fmt.Errorf("request failed, status is %s", resp.Status) + } + + return nil +} + +type File struct { + Branch string `json:"branch"` + AuthorEmail string `json:"author_email"` + AuthorName string `json:"author_name"` + Content checks.GlobalTarget `json:"content"` + CommitMessage string `json:"commit_message"` + fileName string +} + +// Bytes returns the bytes of the File +func (g File) Bytes() ([]byte, error) { + b, err := json.Marshal(g) + return b, err +} diff --git a/pkg/sparrow/gitlab/gitlab_test.go b/pkg/sparrow/gitlab/gitlab_test.go new file mode 100644 index 00000000..327f404f --- /dev/null +++ b/pkg/sparrow/gitlab/gitlab_test.go @@ -0,0 +1,403 @@ +package gitlab + +import ( + "context" + "fmt" + "net/http" + "reflect" + "testing" + "time" + + "github.com/caas-team/sparrow/pkg/checks" + "github.com/jarcoal/httpmock" +) + +func Test_gitlab_fetchFileList(t *testing.T) { + type file struct { + Name string `json:"name"` + } + tests := []struct { + name string + want []string + wantErr bool + mockBody []file + mockCode int + }{ + { + name: "success - 0 targets", + want: nil, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{}, + }, + { + name: "success - 1 target", + want: []string{ + "test", + }, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{ + { + Name: "test", + }, + }, + }, + { + name: "success - 2 targets", + want: []string{ + "test", + "test2", + }, + wantErr: false, + mockCode: http.StatusOK, + mockBody: []file{ + { + Name: "test", + }, + { + Name: "test2", + }, + }, + }, + { + name: "failure - API error", + want: nil, + wantErr: true, + mockCode: http.StatusInternalServerError, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.mockBody) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", "http://test/api/v4/projects/1/repository/tree?ref=main", resp) + + g := &Client{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + got, err := g.fetchFileList(context.Background()) + if (err != nil) != tt.wantErr { + t.Fatalf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("FetchFiles() got = %v, want %v", got, tt.want) + } + }) + } +} + +// The filelist and url are the same, so we HTTP responders can +// be created without much hassle +func Test_gitlab_fetchFiles(t *testing.T) { + tests := []struct { + name string + want []checks.GlobalTarget + fileList []string + wantErr bool + mockCode int + }{ + { + name: "success - 0 targets", + want: nil, + wantErr: false, + mockCode: http.StatusOK, + }, + { + name: "success - 1 target", + want: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + fileList: []string{ + "test", + }, + wantErr: false, + mockCode: http.StatusOK, + }, + { + name: "success - 2 targets", + want: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + { + Url: "test2", + LastSeen: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), + }, + }, + fileList: []string{ + "test", + "test2", + }, + wantErr: false, + mockCode: http.StatusOK, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &Client{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup mock responses + for i, target := range tt.want { + resp, err := httpmock.NewJsonResponder(tt.mockCode, target) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + } + + got, err := g.fetchFiles(context.Background(), tt.fileList) + if (err != nil) != tt.wantErr { + t.Fatalf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("FetchFiles() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_gitlab_fetchFiles_error_cases(t *testing.T) { + type mockResponses struct { + response checks.GlobalTarget + err bool + } + + tests := []struct { + name string + mockResponses []mockResponses + fileList []string + }{ + { + name: "failure - direct API error", + mockResponses: []mockResponses{ + { + err: true, + }, + }, + fileList: []string{ + "test", + }, + }, + { + name: "failure - API error after one successful request", + mockResponses: []mockResponses{ + { + response: checks.GlobalTarget{ + Url: "test", + LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), + }, + err: false, + }, + { + response: checks.GlobalTarget{}, + err: true, + }, + }, + fileList: []string{ + "test", + "test2-will-fail", + }, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &Client{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for i, target := range tt.mockResponses { + if target.err { + errResp := httpmock.NewStringResponder(http.StatusInternalServerError, "") + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), errResp) + continue + } + resp, err := httpmock.NewJsonResponder(http.StatusOK, target) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + } + + _, err := g.fetchFiles(context.Background(), tt.fileList) + if err == nil { + t.Fatalf("Expected error but got none.") + } + }) + } +} + +func TestClient_PutFile(t *testing.T) { + now := time.Now() + tests := []struct { + name string + file File + mockCode int + wantErr bool + }{ + { + name: "success", + file: File{ + Branch: "main", + AuthorEmail: "test@sparrow", + AuthorName: "sparrpw", + Content: checks.GlobalTarget{ + Url: "https://test.de", + LastSeen: now, + }, + CommitMessage: "test-commit", + fileName: "test.de.json", + }, + mockCode: http.StatusOK, + }, + { + name: "failure - API error", + file: File{ + Branch: "main", + AuthorEmail: "test@sparrow", + AuthorName: "sparrpw", + Content: checks.GlobalTarget{ + Url: "https://test.de", + LastSeen: now, + }, + CommitMessage: "test-commit", + fileName: "test.de.json", + }, + mockCode: http.StatusInternalServerError, + wantErr: true, + }, + { + name: "failure - empty file", + wantErr: true, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &Client{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.wantErr { + resp := httpmock.NewStringResponder(tt.mockCode, "") + httpmock.RegisterResponder("PUT", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s", tt.file.fileName), resp) + } else { + resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.file) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("PUT", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s", tt.file.fileName), resp) + } + + if err := g.PutFile(context.Background(), tt.file); (err != nil) != tt.wantErr { + t.Fatalf("PutFile() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestClient_PostFile(t *testing.T) { + now := time.Now() + tests := []struct { + name string + file File + mockCode int + wantErr bool + }{ + { + name: "success", + file: File{ + Branch: "main", + AuthorEmail: "test@sparrow", + AuthorName: "sparrpw", + Content: checks.GlobalTarget{ + Url: "https://test.de", + LastSeen: now, + }, + CommitMessage: "test-commit", + fileName: "test.de.json", + }, + mockCode: http.StatusCreated, + }, + { + name: "failure - API error", + file: File{ + Branch: "main", + AuthorEmail: "test@sparrow", + AuthorName: "sparrpw", + Content: checks.GlobalTarget{ + Url: "https://test.de", + LastSeen: now, + }, + CommitMessage: "test-commit", + fileName: "test.de.json", + }, + mockCode: http.StatusInternalServerError, + wantErr: true, + }, + { + name: "failure - empty file", + wantErr: true, + }, + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + g := &Client{ + baseUrl: "http://test", + projectID: 1, + token: "test", + client: http.DefaultClient, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.wantErr { + resp := httpmock.NewStringResponder(tt.mockCode, "") + httpmock.RegisterResponder("POST", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s", tt.file.fileName), resp) + } else { + resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.file) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("POST", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s", tt.file.fileName), resp) + } + + if err := g.PostFile(context.Background(), tt.file); (err != nil) != tt.wantErr { + t.Fatalf("PostFile() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/pkg/sparrow/gitlab/test/mockclient.go b/pkg/sparrow/gitlab/test/mockclient.go new file mode 100644 index 00000000..bd00e45f --- /dev/null +++ b/pkg/sparrow/gitlab/test/mockclient.go @@ -0,0 +1,38 @@ +package gitlabmock + +import ( + "context" + "fmt" + + "github.com/caas-team/sparrow/pkg/checks" + "github.com/caas-team/sparrow/pkg/sparrow/gitlab" +) + +type MockClient struct { + targets []checks.GlobalTarget + err error +} + +func (m MockClient) PutFile(ctx context.Context, file gitlab.File) error { + panic("implement me") +} + +func (m MockClient) PostFile(ctx context.Context, f gitlab.File) error { + panic("implement me") +} + +func (m MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { + return m.targets, m.err +} + +// New creates a new MockClient to mock Gitlab interaction +func New(targets []checks.GlobalTarget, err bool) gitlab.Gitlab { + var e error + if err { + e = fmt.Errorf("error") + } + return &MockClient{ + targets: targets, + err: e, + } +} diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 92199f62..c475e57e 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -24,12 +24,14 @@ import ( "net/http" "time" + "github.com/caas-team/sparrow/pkg/sparrow/gitlab" + targets "github.com/caas-team/sparrow/pkg/sparrow/targets" + "github.com/caas-team/sparrow/internal/logger" "github.com/caas-team/sparrow/pkg/api" "github.com/caas-team/sparrow/pkg/checks" "github.com/caas-team/sparrow/pkg/config" "github.com/caas-team/sparrow/pkg/db" - targets "github.com/caas-team/sparrow/pkg/sparrow/targets" "github.com/go-chi/chi/v5" ) @@ -63,7 +65,7 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), - targets: targets.NewGitlabManager(targets.NewGitlab("targetsRepo", "gitlabToken"), 5*time.Minute, 15*time.Minute), + targets: targets.NewGitlabManager(gitlab.New("targetsRepo", "gitlabToken", 1), 5*time.Minute, 15*time.Minute), } sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) @@ -187,3 +189,9 @@ func fanInResults(checkChan chan checks.Result, cResult chan checks.ResultDTO, n } } } + +// GlobalTarget represents a GlobalTarget that can be checked +type GlobalTarget struct { + Url string `json:"url"` + LastSeen time.Time `json:"lastSeen"` +} diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index 3e660da3..5e6b5694 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -1,45 +1,22 @@ package targets import ( - "bytes" "context" - "encoding/json" "fmt" - "net/http" - "net/url" "time" - "github.com/caas-team/sparrow/internal/logger" -) + "github.com/caas-team/sparrow/pkg/checks" + "github.com/caas-team/sparrow/pkg/sparrow/gitlab" -var ( - _ Gitlab = &gitlab{} - _ TargetManager = &gitlabTargetManager{} + "github.com/caas-team/sparrow/internal/logger" ) -// Gitlab handles interaction with a gitlab repository containing -// the global targets for the Sparrow instance -type Gitlab interface { - FetchFiles(ctx context.Context) ([]globalTarget, error) - PutFile(ctx context.Context, file GitlabFile) error - PostFile(ctx context.Context, file GitlabFile) error -} - -// gitlab implements Gitlab -type gitlab struct { - // the base URL of the gitlab instance - baseUrl string - // the ID of the project containing the global targets - projectID int - // the token used to authenticate with the gitlab instance - token string - client *http.Client -} +var _ TargetManager = &gitlabTargetManager{} // gitlabTargetManager implements TargetManager type gitlabTargetManager struct { - targets []globalTarget - gitlab Gitlab + targets []checks.GlobalTarget + gitlab gitlab.Gitlab // the DNS name used for self-registration name string // the interval for the target reconciliation process @@ -54,38 +31,24 @@ type gitlabTargetManager struct { } // NewGitlabManager creates a new gitlabTargetManager -func NewGitlabManager(g Gitlab, checkInterval, unhealthyThreshold time.Duration) TargetManager { +func NewGitlabManager(g gitlab.Gitlab, checkInterval, unhealthyThreshold time.Duration) TargetManager { return &gitlabTargetManager{ - targets: []globalTarget{}, gitlab: g, checkInterval: checkInterval, unhealthyThreshold: unhealthyThreshold, } } -// file represents a file in a gitlab repository -type file struct { - Name string `json:"name"` -} - -func NewGitlab(url, token string) Gitlab { - return &gitlab{ - baseUrl: url, - token: token, - client: &http.Client{}, - } -} - // updateRegistration registers the current instance as a global target func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered) log.Debug("Updating registration") - f := GitlabFile{ + f := gitlab.File{ Branch: "main", AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), AuthorName: t.name, - Content: globalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, + Content: checks.GlobalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, } if t.registered { @@ -146,7 +109,7 @@ func (t *gitlabTargetManager) Reconcile(ctx context.Context) { } // GetTargets returns the current targets of the gitlabTargetManager -func (t *gitlabTargetManager) GetTargets() []globalTarget { +func (t *gitlabTargetManager) GetTargets() []checks.GlobalTarget { return t.targets } @@ -154,7 +117,7 @@ func (t *gitlabTargetManager) GetTargets() []globalTarget { // with the latest available healthy targets func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { log := logger.FromContext(ctx).With("name", "updateGlobalTargets") - var healthyTargets []globalTarget + var healthyTargets []checks.GlobalTarget targets, err := t.gitlab.FetchFiles(ctx) if err != nil { @@ -174,208 +137,3 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { log.Debug("Updated global targets", "targets", len(t.targets)) return nil } - -// FetchFiles fetches the files from the global targets repository from the configured gitlab repository -func (g *gitlab) FetchFiles(ctx context.Context) ([]globalTarget, error) { - log := logger.FromContext(ctx).With("name", "FetchFiles") - fl, err := g.fetchFileList(ctx) - if err != nil { - log.Error("Failed to fetch files", "error", err) - return nil, err - } - - result, err := g.fetchFiles(ctx, fl) - if err != nil { - log.Error("Failed to fetch files", "error", err) - return nil, err - } - log.Info("Successfully fetched all target files", "files", len(result)) - return result, nil -} - -// fetchFiles fetches the files from the global targets repository from the configured gitlab repository -func (g *gitlab) fetchFiles(ctx context.Context, fl []string) ([]globalTarget, error) { - var result []globalTarget - log := logger.FromContext(ctx).With("name", "fetchFiles") - log.Debug("Fetching global files") - for _, f := range fl { - // URL encode the name - n := url.PathEscape(f) - req, err := http.NewRequestWithContext(ctx, - http.MethodGet, - fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s/raw?ref=main", g.baseUrl, g.projectID, n), - http.NoBody, - ) - if err != nil { - log.Error("Failed to create request", "error", err) - return nil, err - } - req.Header.Add("PRIVATE-TOKEN", g.token) - req.Header.Add("Content-Type", "application/json") - - res, err := g.client.Do(req) - if err != nil { - log.Error("Failed to fetch file", "file", f, "error", err) - return nil, err - } - if res.StatusCode != http.StatusOK { - log.Error("Failed to fetch file", "status", res.Status) - return nil, fmt.Errorf("request failed, status is %s", res.Status) - } - - defer res.Body.Close() - var gt globalTarget - err = json.NewDecoder(res.Body).Decode(>) - if err != nil { - log.Error("Failed to decode file after fetching", "file", f, "error", err) - return nil, err - } - - log.Debug("Successfully fetched file", "file", f) - result = append(result, gt) - } - return result, nil -} - -// fetchFileList fetches the files from the global targets repository from the configured gitlab repository -func (g *gitlab) fetchFileList(ctx context.Context) ([]string, error) { - log := logger.FromContext(ctx).With("name", "fetchFileList") - log.Debug("Fetching global files") - type file struct { - Name string `json:"name"` - } - - req, err := http.NewRequestWithContext(ctx, - http.MethodGet, - fmt.Sprintf("%s/api/v4/projects/%d/repository/tree?ref=main", g.baseUrl, g.projectID), - http.NoBody, - ) - if err != nil { - log.Error("Failed to create request", "error", err) - return nil, err - } - - req.Header.Add("PRIVATE-TOKEN", g.token) - req.Header.Add("Content-Type", "application/json") - - res, err := g.client.Do(req) - if err != nil { - log.Error("Failed to fetch file list", "error", err) - return nil, err - } - if res.StatusCode != http.StatusOK { - log.Error("Failed to fetch file list", "status", res.Status) - return nil, fmt.Errorf("request failed, status is %s", res.Status) - } - - defer res.Body.Close() - var fl []file - err = json.NewDecoder(res.Body).Decode(&fl) - if err != nil { - log.Error("Failed to decode file list", "error", err) - return nil, err - } - - var result []string - for _, f := range fl { - result = append(result, f.Name) - } - - log.Debug("Successfully fetched file list", "files", len(result)) - return result, nil -} - -type GitlabFile struct { - Branch string `json:"branch"` - AuthorEmail string `json:"author_email"` - AuthorName string `json:"author_name"` - Content globalTarget `json:"content"` - CommitMessage string `json:"commit_message"` - fileName string -} - -// Bytes returns the bytes of the GitlabFile -func (g GitlabFile) Bytes() ([]byte, error) { - b, err := json.Marshal(g) - return b, err -} - -// PutFile commits the current instance to the configured gitlab repository -// as a global target for other sparrow instances to discover -func (g *gitlab) PutFile(ctx context.Context, body GitlabFile) error { - log := logger.FromContext(ctx).With("name", "AddRegistration") - log.Debug("Registering sparrow instance to gitlab") - - // chose method based on whether the registration has already happened - n := url.PathEscape(body.Content.Url) - b, err := body.Bytes() - if err != nil { - log.Error("Failed to create request", "error", err) - return err - } - req, err := http.NewRequestWithContext(ctx, - http.MethodGet, - fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), - bytes.NewBuffer(b), - ) - if err != nil { - log.Error("Failed to create request", "error", err) - return err - } - - req.Header.Add("PRIVATE-TOKEN", g.token) - req.Header.Add("Content-Type", "application/json") - - resp, err := g.client.Do(req) - if err != nil { - log.Error("Failed to push registration file", "error", err) - return err - } - - if resp.StatusCode != http.StatusAccepted { - log.Error("Failed to push registration file", "status", resp.Status) - return fmt.Errorf("request failed, status is %s", resp.Status) - } - - return nil -} - -// PostFile commits the current instance to the configured gitlab repository -// as a global target for other sparrow instances to discover -func (g *gitlab) PostFile(ctx context.Context, body GitlabFile) error { - log := logger.FromContext(ctx).With("name", "AddRegistration") - log.Debug("Registering sparrow instance to gitlab") - - // chose method based on whether the registration has already happened - n := url.PathEscape(body.Content.Url) - b, err := body.Bytes() - if err != nil { - log.Error("Failed to create request", "error", err) - return err - } - req, err := http.NewRequestWithContext(ctx, - http.MethodPost, - fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s", g.baseUrl, g.projectID, n), - bytes.NewBuffer(b), - ) - if err != nil { - log.Error("Failed to create request", "error", err) - return err - } - - req.Header.Add("PRIVATE-TOKEN", g.token) - req.Header.Add("Content-Type", "application/json") - - resp, err := g.client.Do(req) - if err != nil { - log.Error("Failed to push registration file", "error", err) - return err - } - - if resp.StatusCode != http.StatusCreated { - log.Error("Failed to push registration file", "status", resp.Status) - return fmt.Errorf("request failed, status is %s", resp.Status) - } - - return nil -} diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index eebe7313..25fb90c4 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -2,282 +2,37 @@ package targets import ( "context" - "fmt" - "net/http" - "reflect" "testing" "time" - "github.com/jarcoal/httpmock" + "github.com/caas-team/sparrow/pkg/checks" + gitlabmock "github.com/caas-team/sparrow/pkg/sparrow/gitlab/test" ) -func Test_gitlab_fetchFileList(t *testing.T) { - type file struct { - Name string `json:"name"` - } - tests := []struct { - name string - want []string - wantErr bool - mockBody []file - mockCode int - }{ - { - name: "success - 0 targets", - want: nil, - wantErr: false, - mockCode: http.StatusOK, - mockBody: []file{}, - }, - { - name: "success - 1 target", - want: []string{ - "test", - }, - wantErr: false, - mockCode: http.StatusOK, - mockBody: []file{ - { - Name: "test", - }, - }, - }, - { - name: "success - 2 targets", - want: []string{ - "test", - "test2", - }, - wantErr: false, - mockCode: http.StatusOK, - mockBody: []file{ - { - Name: "test", - }, - { - Name: "test2", - }, - }, - }, - { - name: "failure - API error", - want: nil, - wantErr: true, - mockCode: http.StatusInternalServerError, - }, - } - - httpmock.Activate() - defer httpmock.DeactivateAndReset() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.mockBody) - if err != nil { - t.Fatalf("error creating mock response: %v", err) - } - httpmock.RegisterResponder("GET", "http://test/api/v4/projects/1/repository/tree?ref=main", resp) - - g := &gitlab{ - baseUrl: "http://test", - projectID: 1, - token: "test", - client: http.DefaultClient, - } - got, err := g.fetchFileList(context.Background()) - if (err != nil) != tt.wantErr { - t.Errorf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("FetchFiles() got = %v, want %v", got, tt.want) - } - }) - } -} - -// The filelist and url are the same, so we HTTP responders can -// be created without much hassle -func Test_gitlab_fetchFiles(t *testing.T) { - tests := []struct { - name string - want []globalTarget - fileList []string - wantErr bool - mockCode int - }{ - { - name: "success - 0 targets", - want: nil, - wantErr: false, - mockCode: http.StatusOK, - }, - { - name: "success - 1 target", - want: []globalTarget{ - { - Url: "test", - LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), - }, - }, - fileList: []string{ - "test", - }, - wantErr: false, - mockCode: http.StatusOK, - }, - { - name: "success - 2 targets", - want: []globalTarget{ - { - Url: "test", - LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), - }, - { - Url: "test2", - LastSeen: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), - }, - }, - fileList: []string{ - "test", - "test2", - }, - wantErr: false, - mockCode: http.StatusOK, - }, - } - - httpmock.Activate() - defer httpmock.DeactivateAndReset() - g := &gitlab{ - baseUrl: "http://test", - projectID: 1, - token: "test", - client: http.DefaultClient, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // setup mock responses - for i, target := range tt.want { - resp, err := httpmock.NewJsonResponder(tt.mockCode, target) - if err != nil { - t.Fatalf("error creating mock response: %v", err) - } - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) - } - - got, err := g.fetchFiles(context.Background(), tt.fileList) - if (err != nil) != tt.wantErr { - t.Fatalf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) - } - if !reflect.DeepEqual(got, tt.want) { - t.Fatalf("FetchFiles() got = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_gitlab_fetchFiles_error_cases(t *testing.T) { - type mockResponses struct { - response globalTarget - err bool - } - - tests := []struct { - name string - mockResponses []mockResponses - fileList []string - }{ - { - name: "failure - direct API error", - mockResponses: []mockResponses{ - { - err: true, - }, - }, - fileList: []string{ - "test", - }, - }, - { - name: "failure - API error after one successful request", - mockResponses: []mockResponses{ - { - response: globalTarget{ - Url: "test", - LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), - }, - err: false, - }, - { - response: globalTarget{}, - err: true, - }, - }, - fileList: []string{ - "test", - "test2-will-fail", - }, - }, - } - - httpmock.Activate() - defer httpmock.DeactivateAndReset() - g := &gitlab{ - baseUrl: "http://test", - projectID: 1, - token: "test", - client: http.DefaultClient, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - for i, target := range tt.mockResponses { - if target.err { - errResp := httpmock.NewStringResponder(http.StatusInternalServerError, "") - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), errResp) - continue - } - resp, err := httpmock.NewJsonResponder(http.StatusOK, target) - if err != nil { - t.Fatalf("error creating mock response: %v", err) - } - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) - } - - _, err := g.fetchFiles(context.Background(), tt.fileList) - if err == nil { - t.Fatalf("Expected error but got none.") - } - }) - } -} - func Test_gitlabTargetManager_refreshTargets(t *testing.T) { now := time.Now() tooOld := now.Add(-time.Hour * 2) tests := []struct { name string - mockTargets []globalTarget - expectedHealthy []globalTarget + mockTargets []checks.GlobalTarget + expectedHealthy []checks.GlobalTarget wantErr bool }{ { name: "success with 0 targets", - mockTargets: []globalTarget{}, - expectedHealthy: []globalTarget{}, + mockTargets: []checks.GlobalTarget{}, + expectedHealthy: []checks.GlobalTarget{}, }, { name: "success with 1 healthy target", - mockTargets: []globalTarget{ + mockTargets: []checks.GlobalTarget{ { Url: "test", LastSeen: now, }, }, - expectedHealthy: []globalTarget{ + expectedHealthy: []checks.GlobalTarget{ { Url: "test", LastSeen: now, @@ -286,7 +41,7 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { }, { name: "success with 1 unhealthy target", - mockTargets: []globalTarget{ + mockTargets: []checks.GlobalTarget{ { Url: "test", LastSeen: tooOld, @@ -295,7 +50,7 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { }, { name: "success with 1 healthy and 1 unhealthy targets", - mockTargets: []globalTarget{ + mockTargets: []checks.GlobalTarget{ { Url: "test", LastSeen: now, @@ -305,7 +60,7 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { LastSeen: tooOld, }, }, - expectedHealthy: []globalTarget{ + expectedHealthy: []checks.GlobalTarget{ { Url: "test", LastSeen: now, @@ -323,7 +78,7 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { t.Run(tt.name, func(t *testing.T) { gtm := &gitlabTargetManager{ targets: nil, - gitlab: newMockGitlab(tt.mockTargets, tt.wantErr), + gitlab: gitlabmock.New(tt.mockTargets, tt.wantErr), name: "test", unhealthyThreshold: time.Hour, } @@ -333,35 +88,3 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { }) } } - -type mockGitlab struct { - targets []globalTarget - err error -} - -func (m mockGitlab) PutFile(ctx context.Context, file GitlabFile) error { - panic("implement me") -} - -func (m mockGitlab) PostFile(ctx context.Context, f GitlabFile) error { - panic("implement me") -} - -func (m mockGitlab) FetchFiles(ctx context.Context) ([]globalTarget, error) { - return m.targets, m.err -} - -func (m mockGitlab) FetchFileList(ctx context.Context) ([]string, error) { - panic("implement me") -} - -func newMockGitlab(targets []globalTarget, err bool) Gitlab { - var e error - if err { - e = fmt.Errorf("error") - } - return &mockGitlab{ - targets: targets, - err: e, - } -} diff --git a/pkg/sparrow/targets/targetmanager.go b/pkg/sparrow/targets/targetmanager.go new file mode 100644 index 00000000..d7ac279c --- /dev/null +++ b/pkg/sparrow/targets/targetmanager.go @@ -0,0 +1,14 @@ +package targets + +import ( + "context" + + "github.com/caas-team/sparrow/pkg/checks" +) + +// TargetManager handles the management of globalTargets for +// a Sparrow instance +type TargetManager interface { + Reconcile(ctx context.Context) + GetTargets() []checks.GlobalTarget +} diff --git a/pkg/sparrow/targets/targets.go b/pkg/sparrow/targets/targets.go deleted file mode 100644 index 5f1eb2ad..00000000 --- a/pkg/sparrow/targets/targets.go +++ /dev/null @@ -1,19 +0,0 @@ -package targets - -import ( - "context" - "time" -) - -// globalTarget represents a globalTarget that can be checked -type globalTarget struct { - Url string `json:"url"` - LastSeen time.Time `json:"lastSeen"` -} - -// TargetManager handles the management of globalTargets for -// a Sparrow instance -type TargetManager interface { - Reconcile(ctx context.Context) - GetTargets() []globalTarget -} From 97a48e2d97f39f67e33633ffe72471071dda8e96 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Thu, 14 Dec 2023 14:58:50 +0100 Subject: [PATCH 06/28] test: added more tests and implemented mocks Signed-off-by: Bruno Bressi --- pkg/sparrow/gitlab/gitlab.go | 100 ++++++++++--------- pkg/sparrow/gitlab/gitlab_test.go | 61 ++++++++---- pkg/sparrow/gitlab/test/mockclient.go | 48 +++++++--- pkg/sparrow/run.go | 8 +- pkg/sparrow/targets/gitlab.go | 3 +- pkg/sparrow/targets/gitlab_test.go | 132 +++++++++++++++++++++++++- pkg/sparrow/targets/targetmanager.go | 3 + 7 files changed, 268 insertions(+), 87 deletions(-) diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 7984693a..a63f4219 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "net/url" @@ -31,9 +32,9 @@ type Client struct { client *http.Client } -func New(url, token string, pid int) Gitlab { +func New(baseURL, token string, pid int) Gitlab { return &Client{ - baseUrl: url, + baseUrl: baseURL, token: token, projectID: pid, client: &http.Client{}, @@ -49,60 +50,66 @@ func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) return nil, err } - result, err := g.fetchFiles(ctx, fl) - if err != nil { - log.Error("Failed to fetch files", "error", err) - return nil, err - } - log.Info("Successfully fetched all target files", "files", len(result)) - return result, nil -} - -// fetchFiles fetches the files from the global targets repository from the configured gitlab repository -func (g *Client) fetchFiles(ctx context.Context, fl []string) ([]checks.GlobalTarget, error) { var result []checks.GlobalTarget - log := logger.FromContext(ctx).With("name", "fetchFiles") - log.Debug("Fetching global files") for _, f := range fl { - // URL encode the name - n := url.PathEscape(f) - req, err := http.NewRequestWithContext(ctx, - http.MethodGet, - fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s/raw?ref=main", g.baseUrl, g.projectID, n), - http.NoBody, - ) + gl, err := g.fetchFile(ctx, f) if err != nil { - log.Error("Failed to create request", "error", err) + log.Error("Failed fetching files", f, "error", err) return nil, err } - req.Header.Add("PRIVATE-TOKEN", g.token) - req.Header.Add("Content-Type", "application/json") + result = append(result, gl) + } + log.Info("Successfully fetched all target files", "files", len(result)) + return result, nil +} - res, err := g.client.Do(req) - if err != nil { - log.Error("Failed to fetch file", "file", f, "error", err) - return nil, err - } - if res.StatusCode != http.StatusOK { - log.Error("Failed to fetch file", "status", res.Status) - return nil, fmt.Errorf("request failed, status is %s", res.Status) - } +// fetchFile fetches the file from the global targets repository from the configured gitlab repository +func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, error) { + log := logger.FromContext(ctx) + var res checks.GlobalTarget + // URL encode the name + n := url.PathEscape(f) + req, err := http.NewRequestWithContext(ctx, + http.MethodGet, + fmt.Sprintf("%s/api/v4/projects/%d/repository/files/%s/raw?ref=main", g.baseUrl, g.projectID, n), + http.NoBody, + ) + if err != nil { + log.Error("Failed to create request", "error", err) + return res, err + } + req.Header.Add("PRIVATE-TOKEN", g.token) + req.Header.Add("Content-Type", "application/json") - defer res.Body.Close() - var gt checks.GlobalTarget - err = json.NewDecoder(res.Body).Decode(>) + resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer + if err != nil { + log.Error("Failed to fetch file", "file", f, "error", err) + return res, err + } + if resp.StatusCode != http.StatusOK { + log.Error("Failed to fetch file", "status", resp.Status) + return res, fmt.Errorf("request failed, status is %s", resp.Status) + } + + defer func(Body io.ReadCloser) { + err = Body.Close() if err != nil { - log.Error("Failed to decode file after fetching", "file", f, "error", err) - return nil, err + log.Error("Failed to close response body", "error", err) } + }(resp.Body) - log.Debug("Successfully fetched file", "file", f) - result = append(result, gt) + err = json.NewDecoder(resp.Body).Decode(&res) + if err != nil { + log.Error("Failed to decode file after fetching", "file", f, "error", err) + return res, err } - return result, nil + + log.Debug("Successfully fetched file", "file", f) + return res, nil } -// fetchFileList fetches the files from the global targets repository from the configured gitlab repository +// fetchFileList fetches the filenames from the global targets repository from the configured gitlab repository, +// so they may be fetched individually func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { log := logger.FromContext(ctx).With("name", "fetchFileList") log.Debug("Fetching global files") @@ -152,7 +159,7 @@ func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { // PutFile commits the current instance to the configured gitlab repository // as a global target for other sparrow instances to discover -func (g *Client) PutFile(ctx context.Context, body File) error { +func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl,gocritic // no need to refactor yet log := logger.FromContext(ctx).With("name", "AddRegistration") log.Debug("Registering sparrow instance to gitlab") @@ -194,7 +201,7 @@ func (g *Client) PutFile(ctx context.Context, body File) error { // PostFile commits the current instance to the configured gitlab repository // as a global target for other sparrow instances to discover -func (g *Client) PostFile(ctx context.Context, body File) error { +func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl,gocritic // no need to refactor yet log := logger.FromContext(ctx).With("name", "AddRegistration") log.Debug("Registering sparrow instance to gitlab") @@ -234,6 +241,7 @@ func (g *Client) PostFile(ctx context.Context, body File) error { return nil } +// File represents a File manipulation operation via the Gitlab API type File struct { Branch string `json:"branch"` AuthorEmail string `json:"author_email"` @@ -244,7 +252,7 @@ type File struct { } // Bytes returns the bytes of the File -func (g File) Bytes() ([]byte, error) { +func (g *File) Bytes() ([]byte, error) { b, err := json.Marshal(g) return b, err } diff --git a/pkg/sparrow/gitlab/gitlab_test.go b/pkg/sparrow/gitlab/gitlab_test.go index 327f404f..b2658590 100644 --- a/pkg/sparrow/gitlab/gitlab_test.go +++ b/pkg/sparrow/gitlab/gitlab_test.go @@ -98,11 +98,15 @@ func Test_gitlab_fetchFileList(t *testing.T) { // The filelist and url are the same, so we HTTP responders can // be created without much hassle -func Test_gitlab_fetchFiles(t *testing.T) { +func Test_gitlab_FetchFiles(t *testing.T) { + type file struct { + Name string `json:"name"` + } + tests := []struct { name string want []checks.GlobalTarget - fileList []string + fileList []file wantErr bool mockCode int }{ @@ -120,8 +124,10 @@ func Test_gitlab_fetchFiles(t *testing.T) { LastSeen: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), }, }, - fileList: []string{ - "test", + fileList: []file{ + { + Name: "test", + }, }, wantErr: false, mockCode: http.StatusOK, @@ -138,9 +144,13 @@ func Test_gitlab_fetchFiles(t *testing.T) { LastSeen: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), }, }, - fileList: []string{ - "test", - "test2", + fileList: []file{ + { + Name: "test", + }, + { + Name: "test2", + }, }, wantErr: false, mockCode: http.StatusOK, @@ -164,10 +174,16 @@ func Test_gitlab_fetchFiles(t *testing.T) { if err != nil { t.Fatalf("error creating mock response: %v", err) } - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i].Name), resp) } - got, err := g.fetchFiles(context.Background(), tt.fileList) + resp, err := httpmock.NewJsonResponder(tt.mockCode, tt.fileList) + if err != nil { + t.Fatalf("error creating mock response: %v", err) + } + httpmock.RegisterResponder("GET", "http://test/api/v4/projects/1/repository/tree?ref=main", resp) + + got, err := g.FetchFiles(context.Background()) if (err != nil) != tt.wantErr { t.Fatalf("FetchFiles() error = %v, wantErr %v", err, tt.wantErr) } @@ -179,6 +195,9 @@ func Test_gitlab_fetchFiles(t *testing.T) { } func Test_gitlab_fetchFiles_error_cases(t *testing.T) { + type file struct { + Name string `json:"name"` + } type mockResponses struct { response checks.GlobalTarget err bool @@ -187,7 +206,7 @@ func Test_gitlab_fetchFiles_error_cases(t *testing.T) { tests := []struct { name string mockResponses []mockResponses - fileList []string + fileList []file }{ { name: "failure - direct API error", @@ -196,8 +215,10 @@ func Test_gitlab_fetchFiles_error_cases(t *testing.T) { err: true, }, }, - fileList: []string{ - "test", + fileList: []file{ + { + Name: "test", + }, }, }, { @@ -215,9 +236,9 @@ func Test_gitlab_fetchFiles_error_cases(t *testing.T) { err: true, }, }, - fileList: []string{ - "test", - "test2-will-fail", + fileList: []file{ + {Name: "test"}, + {Name: "test2-will-fail"}, }, }, } @@ -236,17 +257,17 @@ func Test_gitlab_fetchFiles_error_cases(t *testing.T) { for i, target := range tt.mockResponses { if target.err { errResp := httpmock.NewStringResponder(http.StatusInternalServerError, "") - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), errResp) + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i].Name), errResp) continue } resp, err := httpmock.NewJsonResponder(http.StatusOK, target) if err != nil { t.Fatalf("error creating mock response: %v", err) } - httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i]), resp) + httpmock.RegisterResponder("GET", fmt.Sprintf("http://test/api/v4/projects/1/repository/files/%s/raw?ref=main", tt.fileList[i].Name), resp) } - _, err := g.fetchFiles(context.Background(), tt.fileList) + _, err := g.FetchFiles(context.Background()) if err == nil { t.Fatalf("Expected error but got none.") } @@ -254,7 +275,7 @@ func Test_gitlab_fetchFiles_error_cases(t *testing.T) { } } -func TestClient_PutFile(t *testing.T) { +func TestClient_PutFile(t *testing.T) { //nolint:dupl // no need to refactor yet now := time.Now() tests := []struct { name string @@ -328,7 +349,7 @@ func TestClient_PutFile(t *testing.T) { } } -func TestClient_PostFile(t *testing.T) { +func TestClient_PostFile(t *testing.T) { //nolint:dupl // no need to refactor yet now := time.Now() tests := []struct { name string diff --git a/pkg/sparrow/gitlab/test/mockclient.go b/pkg/sparrow/gitlab/test/mockclient.go index bd00e45f..12d83548 100644 --- a/pkg/sparrow/gitlab/test/mockclient.go +++ b/pkg/sparrow/gitlab/test/mockclient.go @@ -2,37 +2,55 @@ package gitlabmock import ( "context" - "fmt" + "github.com/caas-team/sparrow/internal/logger" "github.com/caas-team/sparrow/pkg/checks" "github.com/caas-team/sparrow/pkg/sparrow/gitlab" ) type MockClient struct { - targets []checks.GlobalTarget - err error + targets []checks.GlobalTarget + fetchFilesErr error + putFileErr error + postFileErr error } -func (m MockClient) PutFile(ctx context.Context, file gitlab.File) error { - panic("implement me") +func (m *MockClient) PutFile(ctx context.Context, _ gitlab.File) error { //nolint: gocritic // irrelevant + log := logger.FromContext(ctx).With("name", "MockPutFile") + log.Debug("MockPutFile called", "err", m.putFileErr) + return m.putFileErr } -func (m MockClient) PostFile(ctx context.Context, f gitlab.File) error { - panic("implement me") +func (m *MockClient) PostFile(ctx context.Context, _ gitlab.File) error { //nolint: gocritic // irrelevant + log := logger.FromContext(ctx).With("name", "MockPostFile") + log.Debug("MockPostFile called", "err", m.postFileErr) + return m.postFileErr } -func (m MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { - return m.targets, m.err +func (m *MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { + log := logger.FromContext(ctx).With("name", "MockFetchFiles") + log.Debug("MockFetchFiles called", "targets", len(m.targets), "err", m.fetchFilesErr) + return m.targets, m.fetchFilesErr +} + +// SetFetchFilesErr sets the error returned by FetchFiles +func (m *MockClient) SetFetchFilesErr(err error) { + m.fetchFilesErr = err +} + +// SetPutFileErr sets the error returned by PutFile +func (m *MockClient) SetPutFileErr(err error) { + m.putFileErr = err +} + +// SetPostFileErr sets the error returned by PostFile +func (m *MockClient) SetPostFileErr(err error) { + m.postFileErr = err } // New creates a new MockClient to mock Gitlab interaction -func New(targets []checks.GlobalTarget, err bool) gitlab.Gitlab { - var e error - if err { - e = fmt.Errorf("error") - } +func New(targets []checks.GlobalTarget) *MockClient { return &MockClient{ targets: targets, - err: e, } } diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index c475e57e..8805828a 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -35,6 +35,12 @@ import ( "github.com/go-chi/chi/v5" ) +const ( + gitlabRegistrationProjectID = 1 + globalTargetsCheckInterval = 5 * time.Minute + registrationUnhealthyThreshold = 15 * time.Minute +) + type Sparrow struct { db db.DB // the existing checks @@ -65,7 +71,7 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), - targets: targets.NewGitlabManager(gitlab.New("targetsRepo", "gitlabToken", 1), 5*time.Minute, 15*time.Minute), + targets: targets.NewGitlabManager(gitlab.New("targetsRepo", "gitlabToken", gitlabRegistrationProjectID), globalTargetsCheckInterval, registrationUnhealthyThreshold), } sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index 5e6b5694..e289a71f 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -66,6 +66,7 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { err := t.gitlab.PostFile(ctx, f) if err != nil { log.Error("Failed to register global gitlabTargetManager", "error", err) + return err } log.Debug("Successfully registered") @@ -74,7 +75,7 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { } // Reconcile reconciles the targets of the gitlabTargetManager. -// The global targets are parsed from a remote endpoint. +// The global targets are parsed from a gitlab repository. // // The global targets are evaluated for healthiness and // unhealthy gitlabTargetManager are removed. diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index 25fb90c4..7db745d8 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -2,6 +2,7 @@ package targets import ( "context" + "fmt" "testing" "time" @@ -17,7 +18,7 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { name string mockTargets []checks.GlobalTarget expectedHealthy []checks.GlobalTarget - wantErr bool + wantErr error }{ { name: "success with 0 targets", @@ -71,20 +72,143 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { name: "failure getting targets", mockTargets: nil, expectedHealthy: nil, - wantErr: true, + wantErr: fmt.Errorf("failed to fetch files"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + gitlab := gitlabmock.New(tt.mockTargets) + if tt.wantErr != nil { + gitlab.SetFetchFilesErr(tt.wantErr) + } gtm := &gitlabTargetManager{ targets: nil, - gitlab: gitlabmock.New(tt.mockTargets, tt.wantErr), + gitlab: gitlab, name: "test", unhealthyThreshold: time.Hour, } - if err := gtm.refreshTargets(context.Background()); (err != nil) != tt.wantErr { + if err := gtm.refreshTargets(context.Background()); (err != nil) != (tt.wantErr != nil) { t.Fatalf("refreshTargets() error = %v, wantErr %v", err, tt.wantErr) } }) } } + +func Test_gitlabTargetManager_GetTargets(t *testing.T) { + now := time.Now() + tests := []struct { + name string + targets []checks.GlobalTarget + want []checks.GlobalTarget + }{ + { + name: "success with 0 targets", + targets: nil, + want: nil, + }, + { + name: "success with 1 target", + targets: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: now, + }, + }, + want: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: now, + }, + }, + }, + { + name: "success with 2 targets", + targets: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: now, + }, + { + Url: "test2", + LastSeen: now, + }, + }, + want: []checks.GlobalTarget{ + { + Url: "test", + LastSeen: now, + }, + { + Url: "test2", + LastSeen: now, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gtm := &gitlabTargetManager{ + targets: tt.targets, + } + got := gtm.GetTargets() + + if len(got) != len(tt.want) { + t.Fatalf("GetTargets() got = %v, want %v", got, tt.want) + } + + for i := range got { + if got[i].Url != tt.want[i].Url { + t.Fatalf("GetTargets() got = %v, want %v", got, tt.want) + } + if !got[i].LastSeen.Equal(tt.want[i].LastSeen) { + t.Fatalf("GetTargets() got = %v, want %v", got, tt.want) + } + } + }) + } +} + +func Test_gitlabTargetManager_updateRegistration(t *testing.T) { + tests := []struct { + name string + registered bool + wantPostError bool + wantPutError bool + }{ + { + name: "success - first registration", + }, + { + name: "success - update registration", + registered: true, + }, + { + name: "failure - failed to register", + wantPostError: true, + }, + { + name: "failure - failed to update registration", + registered: true, + wantPutError: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + glmock := gitlabmock.New(nil) + if tt.wantPostError { + glmock.SetPostFileErr(fmt.Errorf("failed to register")) + } + if tt.wantPutError { + glmock.SetPutFileErr(fmt.Errorf("failed to update registration")) + } + gtm := &gitlabTargetManager{ + gitlab: glmock, + registered: tt.registered, + } + wantErr := tt.wantPutError || tt.wantPostError + if err := gtm.updateRegistration(context.Background()); (err != nil) != wantErr { + t.Fatalf("updateRegistration() error = %v, wantErr %v", err, wantErr) + } + }) + } +} diff --git a/pkg/sparrow/targets/targetmanager.go b/pkg/sparrow/targets/targetmanager.go index d7ac279c..d7508b63 100644 --- a/pkg/sparrow/targets/targetmanager.go +++ b/pkg/sparrow/targets/targetmanager.go @@ -9,6 +9,9 @@ import ( // TargetManager handles the management of globalTargets for // a Sparrow instance type TargetManager interface { + // Reconcile fetches the global targets from the configured + // endpoint and updates the local state Reconcile(ctx context.Context) + // GetTargets returns the current global targets GetTargets() []checks.GlobalTarget } From 7a299165cfc1114379754303daf835ed01c59b2e Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Thu, 14 Dec 2023 16:11:38 +0100 Subject: [PATCH 07/28] feat: added shutdown for the target manager Signed-off-by: Bruno Bressi --- pkg/sparrow/gitlab/test/mockclient.go | 12 +- pkg/sparrow/run.go | 9 +- pkg/sparrow/targets/gitlab.go | 137 ++++++++++++++-------- pkg/sparrow/targets/gitlab_test.go | 158 ++++++++++++++++++++++++++ pkg/sparrow/targets/targetmanager.go | 3 + 5 files changed, 266 insertions(+), 53 deletions(-) diff --git a/pkg/sparrow/gitlab/test/mockclient.go b/pkg/sparrow/gitlab/test/mockclient.go index 12d83548..98790898 100644 --- a/pkg/sparrow/gitlab/test/mockclient.go +++ b/pkg/sparrow/gitlab/test/mockclient.go @@ -16,20 +16,20 @@ type MockClient struct { } func (m *MockClient) PutFile(ctx context.Context, _ gitlab.File) error { //nolint: gocritic // irrelevant - log := logger.FromContext(ctx).With("name", "MockPutFile") - log.Debug("MockPutFile called", "err", m.putFileErr) + log := logger.FromContext(ctx) + log.Info("MockPutFile called", "err", m.putFileErr) return m.putFileErr } func (m *MockClient) PostFile(ctx context.Context, _ gitlab.File) error { //nolint: gocritic // irrelevant - log := logger.FromContext(ctx).With("name", "MockPostFile") - log.Debug("MockPostFile called", "err", m.postFileErr) + log := logger.FromContext(ctx) + log.Info("MockPostFile called", "err", m.postFileErr) return m.postFileErr } func (m *MockClient) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { - log := logger.FromContext(ctx).With("name", "MockFetchFiles") - log.Debug("MockFetchFiles called", "targets", len(m.targets), "err", m.fetchFilesErr) + log := logger.FromContext(ctx) + log.Info("MockFetchFiles called", "targets", len(m.targets), "err", m.fetchFilesErr) return m.targets, m.fetchFilesErr } diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 8805828a..0303bd7c 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -39,6 +39,7 @@ const ( gitlabRegistrationProjectID = 1 globalTargetsCheckInterval = 5 * time.Minute registrationUnhealthyThreshold = 15 * time.Minute + registrationInterval = 5 * time.Minute ) type Sparrow struct { @@ -71,7 +72,13 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), - targets: targets.NewGitlabManager(gitlab.New("targetsRepo", "gitlabToken", gitlabRegistrationProjectID), globalTargetsCheckInterval, registrationUnhealthyThreshold), + targets: targets.NewGitlabManager( + gitlab.New("targetsRepo", "gitlabToken", gitlabRegistrationProjectID), + "DNS-Name", + globalTargetsCheckInterval, + registrationUnhealthyThreshold, + registrationInterval, + ), } sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index e289a71f..6804a45e 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -3,6 +3,7 @@ package targets import ( "context" "fmt" + "sync" "time" "github.com/caas-team/sparrow/pkg/checks" @@ -16,6 +17,8 @@ var _ TargetManager = &gitlabTargetManager{} // gitlabTargetManager implements TargetManager type gitlabTargetManager struct { targets []checks.GlobalTarget + mu sync.RWMutex + done chan struct{} gitlab gitlab.Gitlab // the DNS name used for self-registration name string @@ -31,49 +34,18 @@ type gitlabTargetManager struct { } // NewGitlabManager creates a new gitlabTargetManager -func NewGitlabManager(g gitlab.Gitlab, checkInterval, unhealthyThreshold time.Duration) TargetManager { +func NewGitlabManager(g gitlab.Gitlab, name string, checkInterval, unhealthyThreshold, regInterval time.Duration) *gitlabTargetManager { return &gitlabTargetManager{ - gitlab: g, - checkInterval: checkInterval, - unhealthyThreshold: unhealthyThreshold, + gitlab: g, + name: name, + checkInterval: checkInterval, + registrationInterval: regInterval, + unhealthyThreshold: unhealthyThreshold, + mu: sync.RWMutex{}, + done: make(chan struct{}, 1), } } -// updateRegistration registers the current instance as a global target -func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { - log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered) - log.Debug("Updating registration") - - f := gitlab.File{ - Branch: "main", - AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), - AuthorName: t.name, - Content: checks.GlobalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, - } - - if t.registered { - f.CommitMessage = "Updated registration" - err := t.gitlab.PutFile(ctx, f) - if err != nil { - log.Error("Failed to update registration", "error", err) - return err - } - log.Debug("Successfully updated registration") - return nil - } - - f.CommitMessage = "Initial registration" - err := t.gitlab.PostFile(ctx, f) - if err != nil { - log.Error("Failed to register global gitlabTargetManager", "error", err) - return err - } - - log.Debug("Successfully registered") - t.registered = true - return nil -} - // Reconcile reconciles the targets of the gitlabTargetManager. // The global targets are parsed from a gitlab repository. // @@ -83,43 +55,109 @@ func (t *gitlabTargetManager) Reconcile(ctx context.Context) { log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") log.Debug("Starting global gitlabTargetManager reconciler") + checkTimer := time.NewTimer(t.checkInterval) + registrationTimer := time.NewTimer(t.registrationInterval) + + defer checkTimer.Stop() + defer registrationTimer.Stop() + for { select { case <-ctx.Done(): if err := ctx.Err(); err != nil { log.Error("Context canceled", "error", err) - return + err = t.Shutdown(ctx) + if err != nil { + log.Error("Failed to shutdown gracefully", "error", err) + return + } } - // check if this blocks when context is canceled - case <-time.After(t.checkInterval): - log.Debug("Getting global gitlabTargetManager") + case <-t.done: + log.Info("Ending Reconcile routine.") + return + case <-checkTimer.C: err := t.refreshTargets(ctx) if err != nil { log.Error("Failed to get global gitlabTargetManager", "error", err) continue } - case <-time.After(t.registrationInterval): - log.Debug("Registering global gitlabTargetManager") + checkTimer.Reset(t.checkInterval) + case <-registrationTimer.C: err := t.updateRegistration(ctx) if err != nil { log.Error("Failed to register global gitlabTargetManager", "error", err) continue } + registrationTimer.Reset(t.registrationInterval) } } } // GetTargets returns the current targets of the gitlabTargetManager func (t *gitlabTargetManager) GetTargets() []checks.GlobalTarget { + t.mu.RLock() + defer t.mu.RUnlock() return t.targets } +// Shutdown shuts down the gitlabTargetManager and deletes the file containing +// the sparrow's registration from Gitlab +func (t *gitlabTargetManager) Shutdown(ctx context.Context) error { + log := logger.FromContext(ctx).With("name", "Shutdown") + log.Debug("Shutting down global gitlabTargetManager") + t.mu.Lock() + defer t.mu.Unlock() + t.registered = false + t.done <- struct{}{} + return nil +} + +// updateRegistration registers the current instance as a global target +func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { + log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered) + log.Debug("Updating registration") + + f := gitlab.File{ + Branch: "main", + AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), + AuthorName: t.name, + Content: checks.GlobalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, + } + + if t.Registered() { + t.mu.Lock() + defer t.mu.Unlock() + f.CommitMessage = "Updated registration" + err := t.gitlab.PutFile(ctx, f) + if err != nil { + log.Error("Failed to update registration", "error", err) + return err + } + log.Debug("Successfully updated registration") + return nil + } + + t.mu.Lock() + defer t.mu.Unlock() + f.CommitMessage = "Initial registration" + err := t.gitlab.PostFile(ctx, f) + if err != nil { + log.Error("Failed to register global gitlabTargetManager", "error", err) + return err + } + + log.Debug("Successfully registered") + t.registered = true + return nil +} + // refreshTargets updates the targets of the gitlabTargetManager // with the latest available healthy targets func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { log := logger.FromContext(ctx).With("name", "updateGlobalTargets") + t.mu.Lock() var healthyTargets []checks.GlobalTarget - + defer t.mu.Unlock() targets, err := t.gitlab.FetchFiles(ctx) if err != nil { log.Error("Failed to update global targets", "error", err) @@ -129,6 +167,7 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { // filter unhealthy targets - this may be removed in the future for _, target := range targets { if time.Now().Add(-t.unhealthyThreshold).After(target.LastSeen) { + log.Debug("Skipping unhealthy target", "target", target) continue } healthyTargets = append(healthyTargets, target) @@ -138,3 +177,9 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { log.Debug("Updated global targets", "targets", len(t.targets)) return nil } + +func (t *gitlabTargetManager) Registered() bool { + t.mu.RLock() + defer t.mu.RUnlock() + return t.registered +} diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index 7db745d8..7ab7bd55 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -2,6 +2,7 @@ package targets import ( "context" + "errors" "fmt" "testing" "time" @@ -212,3 +213,160 @@ func Test_gitlabTargetManager_updateRegistration(t *testing.T) { }) } } + +// Test_gitlabTargetManager_Reconcile_success tests that the Reconcile method +// will register the target if it is not registered yet and update the +// registration if it is already registered +func Test_gitlabTargetManager_Reconcile_success(t *testing.T) { + tests := []struct { + name string + registered bool + wantPostError bool + wantPutError bool + }{ + { + name: "success - first registration", + }, + { + name: "success - update registration", + registered: true, + }, + } + + glmock := gitlabmock.New( + []checks.GlobalTarget{ + { + Url: "test", + LastSeen: time.Now(), + }, + }, + ) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gtm := NewGitlabManager( + glmock, + "test", + time.Millisecond*100, + time.Hour*1, + time.Millisecond*150, + ) + ctx := context.Background() + go func() { + gtm.Reconcile(ctx) + }() + + time.Sleep(time.Millisecond * 300) + if gtm.GetTargets()[0].Url != "test" { + t.Fatalf("Reconcile() did not receive the correct target") + } + if !gtm.Registered() { + t.Fatalf("Reconcile() did not register") + } + + err := gtm.Shutdown(ctx) + if err != nil { + t.Fatalf("Reconcile() failed to shutdown") + } + }) + } +} + +// Test_gitlabTargetManager_Reconcile_failure tests that the Reconcile method +// will handle API failures gracefully +func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { + tests := []struct { + name string + registered bool + postErr error + putError error + }{ + { + name: "failure - failed to register", + postErr: errors.New("failed to register"), + }, + { + name: "failure - failed to update registration", + registered: true, + putError: errors.New("failed to update registration"), + }, + } + + glmock := gitlabmock.New( + []checks.GlobalTarget{ + { + Url: "test", + LastSeen: time.Now(), + }, + }, + ) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gtm := NewGitlabManager( + glmock, + "test", + time.Millisecond*100, + time.Hour*1, + time.Millisecond*150, + ) + glmock.SetPostFileErr(tt.postErr) + glmock.SetPutFileErr(tt.putError) + + ctx := context.Background() + go func() { + gtm.Reconcile(ctx) + }() + + time.Sleep(time.Millisecond * 300) + + if tt.postErr != nil && gtm.Registered() { + t.Fatalf("Reconcile() should not have registered") + } + + if tt.putError != nil && !gtm.Registered() { + t.Fatalf("Reconcile() should still be registered") + } + + err := gtm.Shutdown(ctx) + if err != nil { + t.Fatalf("Reconcile() failed to shutdown") + } + }) + } +} + +// Test_gitlabTargetManager_Reconcile_Context_Canceled tests that the Reconcile +// method will shutdown gracefully when the context is canceled. +func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { + glmock := gitlabmock.New( + []checks.GlobalTarget{ + { + Url: "test", + LastSeen: time.Now(), + }, + }, + ) + + gtm := NewGitlabManager( + glmock, + "test", + time.Millisecond*100, + time.Hour*1, + time.Millisecond*150, + ) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + gtm.Reconcile(ctx) + }() + + time.Sleep(time.Millisecond * 250) + cancel() + time.Sleep(time.Millisecond * 100) + + // instance shouldn't be registered anymore + if gtm.Registered() { + t.Fatalf("Reconcile() should not be registered") + } +} diff --git a/pkg/sparrow/targets/targetmanager.go b/pkg/sparrow/targets/targetmanager.go index d7508b63..77f37ec4 100644 --- a/pkg/sparrow/targets/targetmanager.go +++ b/pkg/sparrow/targets/targetmanager.go @@ -14,4 +14,7 @@ type TargetManager interface { Reconcile(ctx context.Context) // GetTargets returns the current global targets GetTargets() []checks.GlobalTarget + // Shutdown shuts down the target manager + // and unregisters the instance as a global target + Shutdown(ctx context.Context) error } From fa12911a8587604fbc23289efc9e03f260604809 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Thu, 14 Dec 2023 16:16:19 +0100 Subject: [PATCH 08/28] chore: globaltarget docs Signed-off-by: Bruno Bressi --- pkg/checks/checks.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/checks/checks.go b/pkg/checks/checks.go index b8cbd6b0..513e7c23 100644 --- a/pkg/checks/checks.go +++ b/pkg/checks/checks.go @@ -74,6 +74,8 @@ type Result struct { Err string `json:"error"` } +// GlobalTarget includes the basic information regarding +// other Sparrow instances, which this Sparrow can communicate with. type GlobalTarget struct { Url string `json:"url"` LastSeen time.Time `json:"lastSeen"` From f0b6b3110a5dca124633849909468ae4ceb1ef7e Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Thu, 14 Dec 2023 17:34:23 +0100 Subject: [PATCH 09/28] fix: fixed block on shutdown && removed With() logs Signed-off-by: Bruno Bressi --- .github/workflows/test_unit.yml | 2 +- .pre-commit-config.yaml | 2 +- pkg/sparrow/gitlab/gitlab.go | 8 ++++---- pkg/sparrow/targets/gitlab.go | 25 ++++++++++++++++--------- pkg/sparrow/targets/gitlab_test.go | 2 +- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test_unit.yml b/.github/workflows/test_unit.yml index 3bbdc3d1..a2a6a890 100644 --- a/.github/workflows/test_unit.yml +++ b/.github/workflows/test_unit.yml @@ -25,4 +25,4 @@ jobs: go mod download go install github.com/matryer/moq@v0.3.3 go generate ./... - go test --race --coverprofile cover.out -v ./... \ No newline at end of file + go test --race --count=1 --coverprofile cover.out -v ./... \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 76b30808..d850a9b1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: hooks: - id: go-mod-tidy-repo - id: go-test-repo-mod - args: [ -race ] + args: [ -race, -count=1 ] - id: go-vet-repo-mod - id: go-fumpt-repo args: [ -l, -w ] diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index a63f4219..5527de88 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -43,7 +43,7 @@ func New(baseURL, token string, pid int) Gitlab { // FetchFiles fetches the files from the global targets repository from the configured gitlab repository func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) { - log := logger.FromContext(ctx).With("name", "FetchFiles") + log := logger.FromContext(ctx) fl, err := g.fetchFileList(ctx) if err != nil { log.Error("Failed to fetch files", "error", err) @@ -111,7 +111,7 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, // fetchFileList fetches the filenames from the global targets repository from the configured gitlab repository, // so they may be fetched individually func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { - log := logger.FromContext(ctx).With("name", "fetchFileList") + log := logger.FromContext(ctx) log.Debug("Fetching global files") type file struct { Name string `json:"name"` @@ -160,7 +160,7 @@ func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { // PutFile commits the current instance to the configured gitlab repository // as a global target for other sparrow instances to discover func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl,gocritic // no need to refactor yet - log := logger.FromContext(ctx).With("name", "AddRegistration") + log := logger.FromContext(ctx) log.Debug("Registering sparrow instance to gitlab") // chose method based on whether the registration has already happened @@ -202,7 +202,7 @@ func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl, // PostFile commits the current instance to the configured gitlab repository // as a global target for other sparrow instances to discover func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl,gocritic // no need to refactor yet - log := logger.FromContext(ctx).With("name", "AddRegistration") + log := logger.FromContext(ctx) log.Debug("Registering sparrow instance to gitlab") // chose method based on whether the registration has already happened diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index 6804a45e..c68159eb 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -52,7 +52,7 @@ func NewGitlabManager(g gitlab.Gitlab, name string, checkInterval, unhealthyThre // The global targets are evaluated for healthiness and // unhealthy gitlabTargetManager are removed. func (t *gitlabTargetManager) Reconcile(ctx context.Context) { - log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets") + log := logger.FromContext(ctx) log.Debug("Starting global gitlabTargetManager reconciler") checkTimer := time.NewTimer(t.checkInterval) @@ -103,18 +103,24 @@ func (t *gitlabTargetManager) GetTargets() []checks.GlobalTarget { // Shutdown shuts down the gitlabTargetManager and deletes the file containing // the sparrow's registration from Gitlab func (t *gitlabTargetManager) Shutdown(ctx context.Context) error { - log := logger.FromContext(ctx).With("name", "Shutdown") - log.Debug("Shutting down global gitlabTargetManager") t.mu.Lock() defer t.mu.Unlock() + log := logger.FromContext(ctx) + log.Info("Shutting down global gitlabTargetManager") t.registered = false - t.done <- struct{}{} + + select { + case t.done <- struct{}{}: + log.Debug("Stopping reconcile routine") + default: + } + return nil } // updateRegistration registers the current instance as a global target func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { - log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered) + log := logger.FromContext(ctx) log.Debug("Updating registration") f := gitlab.File{ @@ -154,10 +160,10 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { // refreshTargets updates the targets of the gitlabTargetManager // with the latest available healthy targets func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { - log := logger.FromContext(ctx).With("name", "updateGlobalTargets") + log := logger.FromContext(ctx) t.mu.Lock() - var healthyTargets []checks.GlobalTarget defer t.mu.Unlock() + var healthyTargets []checks.GlobalTarget targets, err := t.gitlab.FetchFiles(ctx) if err != nil { log.Error("Failed to update global targets", "error", err) @@ -178,8 +184,9 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { return nil } +// Registered returns whether the instance is registered as a global target func (t *gitlabTargetManager) Registered() bool { - t.mu.RLock() - defer t.mu.RUnlock() + t.mu.Lock() + defer t.mu.Unlock() return t.registered } diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index 7ab7bd55..fce9400c 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -363,7 +363,7 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { time.Sleep(time.Millisecond * 250) cancel() - time.Sleep(time.Millisecond * 100) + time.Sleep(time.Millisecond * 250) // instance shouldn't be registered anymore if gtm.Registered() { From 6a1d04149626ae7c9a153138784829d47ee05639 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Thu, 14 Dec 2023 17:52:33 +0100 Subject: [PATCH 10/28] debug: added variables to make the sparrow testable after building Signed-off-by: Bruno Bressi --- pkg/sparrow/run.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 0303bd7c..83eb4fbb 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "net/http" + "os" "time" "github.com/caas-team/sparrow/pkg/sparrow/gitlab" @@ -36,10 +37,11 @@ import ( ) const ( - gitlabRegistrationProjectID = 1 + gitlabRegistrationProjectID = 237078 globalTargetsCheckInterval = 5 * time.Minute registrationUnhealthyThreshold = 15 * time.Minute registrationInterval = 5 * time.Minute + gitlabBaseUrl = "https://gitlab.devops.telekom.de" ) type Sparrow struct { @@ -73,8 +75,8 @@ func New(cfg *config.Config) *Sparrow { routingTree: api.NewRoutingTree(), router: chi.NewRouter(), targets: targets.NewGitlabManager( - gitlab.New("targetsRepo", "gitlabToken", gitlabRegistrationProjectID), - "DNS-Name", + gitlab.New(gitlabBaseUrl, os.Getenv("GITLAB_TOKEN"), gitlabRegistrationProjectID), + "cool-sparrow.de", globalTargetsCheckInterval, registrationUnhealthyThreshold, registrationInterval, From 6fcea509973d22c6066da89dc48e878f10cafff1 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 09:19:25 +0100 Subject: [PATCH 11/28] chore: remove generate of moq files for tests Signed-off-by: Bruno Bressi --- .github/workflows/test_unit.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/test_unit.yml b/.github/workflows/test_unit.yml index a2a6a890..5f8018f1 100644 --- a/.github/workflows/test_unit.yml +++ b/.github/workflows/test_unit.yml @@ -23,6 +23,4 @@ jobs: - name: Test run: | go mod download - go install github.com/matryer/moq@v0.3.3 - go generate ./... go test --race --count=1 --coverprofile cover.out -v ./... \ No newline at end of file From 89f799fb73632aed63951b9225082e1019410428 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 15:15:50 +0100 Subject: [PATCH 12/28] System testing changes implemented: - feat: marking sparrow as registered after found own URL in targets - fix: encoding of File's Content field for API - chore: codebase cleanup, logs & linting --- pkg/checks/checks.go | 1 + pkg/sparrow/gitlab/gitlab.go | 59 ++++++++++++++++++++------ pkg/sparrow/targets/gitlab.go | 21 +++++---- pkg/sparrow/targets/gitlab_test.go | 68 ++++++++++++++++++------------ 4 files changed, 98 insertions(+), 51 deletions(-) diff --git a/pkg/checks/checks.go b/pkg/checks/checks.go index 513e7c23..98856e14 100644 --- a/pkg/checks/checks.go +++ b/pkg/checks/checks.go @@ -80,6 +80,7 @@ type GlobalTarget struct { Url string `json:"url"` LastSeen time.Time `json:"lastSeen"` } + type ResultDTO struct { Name string Result *Result diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 5527de88..6b7f8300 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -3,6 +3,7 @@ package gitlab import ( "bytes" "context" + "encoding/base64" "encoding/json" "fmt" "io" @@ -54,7 +55,7 @@ func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) for _, f := range fl { gl, err := g.fetchFile(ctx, f) if err != nil { - log.Error("Failed fetching files", f, "error", err) + log.Error("Failed fetching files", "error", err) return nil, err } result = append(result, gl) @@ -112,7 +113,7 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, // so they may be fetched individually func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { log := logger.FromContext(ctx) - log.Debug("Fetching global files") + log.Debug("Fetching file list from gitlab") type file struct { Name string `json:"name"` } @@ -183,13 +184,18 @@ func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl, req.Header.Add("PRIVATE-TOKEN", g.token) req.Header.Add("Content-Type", "application/json") - resp, err := g.client.Do(req) + resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer if err != nil { log.Error("Failed to push registration file", "error", err) return err } - defer resp.Body.Close() + defer func(Body io.ReadCloser) { + err = Body.Close() + if err != nil { + log.Error("Failed to close response body", "error", err) + } + }(resp.Body) if resp.StatusCode != http.StatusOK { log.Error("Failed to push registration file", "status", resp.Status) @@ -203,7 +209,7 @@ func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl, // as a global target for other sparrow instances to discover func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl,gocritic // no need to refactor yet log := logger.FromContext(ctx) - log.Debug("Registering sparrow instance to gitlab") + log.Debug("Posting registration file to gitlab") // chose method based on whether the registration has already happened n := url.PathEscape(body.fileName) @@ -225,16 +231,21 @@ func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl, req.Header.Add("PRIVATE-TOKEN", g.token) req.Header.Add("Content-Type", "application/json") - resp, err := g.client.Do(req) + resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer if err != nil { - log.Error("Failed to push registration file", "error", err) + log.Error("Failed to post file", "error", err) return err } - defer resp.Body.Close() + defer func(Body io.ReadCloser) { + err = Body.Close() + if err != nil { + log.Error("Failed to close response body", "error", err) + } + }(resp.Body) if resp.StatusCode != http.StatusCreated { - log.Error("Failed to push registration file", "status", resp.Status) + log.Error("Failed to post file", "status", resp.Status) return fmt.Errorf("request failed, status is %s", resp.Status) } @@ -251,8 +262,32 @@ type File struct { fileName string } -// Bytes returns the bytes of the File +// Bytes returns the File as a byte array. The Content +// is base64 encoded for Gitlab API compatibility. func (g *File) Bytes() ([]byte, error) { - b, err := json.Marshal(g) - return b, err + content, err := json.Marshal(g.Content) + if err != nil { + return nil, err + } + + // base64 encode the content + enc := base64.NewEncoder(base64.StdEncoding, bytes.NewBuffer(content)) + _, err = enc.Write(content) + _ = enc.Close() + + if err != nil { + return nil, err + } + return json.Marshal(map[string]string{ + "branch": g.Branch, + "author_email": g.AuthorEmail, + "author_name": g.AuthorName, + "content": string(content), + "commit_message": g.CommitMessage, + }) +} + +// SetFileName sets the filename of the File +func (g *File) SetFileName(name string) { + g.fileName = name } diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index c68159eb..a535a0d8 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -53,7 +53,7 @@ func NewGitlabManager(g gitlab.Gitlab, name string, checkInterval, unhealthyThre // unhealthy gitlabTargetManager are removed. func (t *gitlabTargetManager) Reconcile(ctx context.Context) { log := logger.FromContext(ctx) - log.Debug("Starting global gitlabTargetManager reconciler") + log.Info("Starting global gitlabTargetManager reconciler") checkTimer := time.NewTimer(t.checkInterval) registrationTimer := time.NewTimer(t.registrationInterval) @@ -78,15 +78,13 @@ func (t *gitlabTargetManager) Reconcile(ctx context.Context) { case <-checkTimer.C: err := t.refreshTargets(ctx) if err != nil { - log.Error("Failed to get global gitlabTargetManager", "error", err) - continue + log.Error("Failed to get global targets", "error", err) } checkTimer.Reset(t.checkInterval) case <-registrationTimer.C: err := t.updateRegistration(ctx) if err != nil { - log.Error("Failed to register global gitlabTargetManager", "error", err) - continue + log.Error("Failed to register self as global target", "error", err) } registrationTimer.Reset(t.registrationInterval) } @@ -123,16 +121,17 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { log := logger.FromContext(ctx) log.Debug("Updating registration") + t.mu.Lock() + defer t.mu.Unlock() f := gitlab.File{ Branch: "main", AuthorEmail: fmt.Sprintf("%s@sparrow", t.name), AuthorName: t.name, Content: checks.GlobalTarget{Url: fmt.Sprintf("https://%s", t.name), LastSeen: time.Now().UTC()}, } + f.SetFileName(fmt.Sprintf("%s.json", t.name)) if t.Registered() { - t.mu.Lock() - defer t.mu.Unlock() f.CommitMessage = "Updated registration" err := t.gitlab.PutFile(ctx, f) if err != nil { @@ -143,8 +142,6 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error { return nil } - t.mu.Lock() - defer t.mu.Unlock() f.CommitMessage = "Initial registration" err := t.gitlab.PostFile(ctx, f) if err != nil { @@ -172,6 +169,10 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { // filter unhealthy targets - this may be removed in the future for _, target := range targets { + if !t.Registered() && target.Url == fmt.Sprintf("https://%s", t.name) { + log.Info("Found self as global target", "lastSeenMin", time.Since(target.LastSeen).Minutes()) + t.registered = true + } if time.Now().Add(-t.unhealthyThreshold).After(target.LastSeen) { log.Debug("Skipping unhealthy target", "target", target) continue @@ -186,7 +187,5 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { // Registered returns whether the instance is registered as a global target func (t *gitlabTargetManager) Registered() bool { - t.mu.Lock() - defer t.mu.Unlock() return t.registered } diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index fce9400c..0ad4b7de 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -16,10 +16,11 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { tooOld := now.Add(-time.Hour * 2) tests := []struct { - name string - mockTargets []checks.GlobalTarget - expectedHealthy []checks.GlobalTarget - wantErr error + name string + mockTargets []checks.GlobalTarget + expectedHealthy []checks.GlobalTarget + expectedRegisteredAfter bool + wantErr error }{ { name: "success with 0 targets", @@ -30,44 +31,47 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { name: "success with 1 healthy target", mockTargets: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, }, expectedHealthy: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, }, + expectedRegisteredAfter: true, }, { name: "success with 1 unhealthy target", mockTargets: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: tooOld, }, }, + expectedRegisteredAfter: true, }, { name: "success with 1 healthy and 1 unhealthy targets", mockTargets: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, { - Url: "test2", + Url: "https://test2", LastSeen: tooOld, }, }, expectedHealthy: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, }, + expectedRegisteredAfter: true, }, { name: "failure getting targets", @@ -91,6 +95,10 @@ func Test_gitlabTargetManager_refreshTargets(t *testing.T) { if err := gtm.refreshTargets(context.Background()); (err != nil) != (tt.wantErr != nil) { t.Fatalf("refreshTargets() error = %v, wantErr %v", err, tt.wantErr) } + + if gtm.Registered() != tt.expectedRegisteredAfter { + t.Fatalf("expected registered to be %v, got %v", tt.expectedRegisteredAfter, gtm.Registered()) + } }) } } @@ -111,13 +119,13 @@ func Test_gitlabTargetManager_GetTargets(t *testing.T) { name: "success with 1 target", targets: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, }, want: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, }, @@ -126,21 +134,21 @@ func Test_gitlabTargetManager_GetTargets(t *testing.T) { name: "success with 2 targets", targets: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, { - Url: "test2", + Url: "https://test2", LastSeen: now, }, }, want: []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: now, }, { - Url: "test2", + Url: "https://test2", LastSeen: now, }, }, @@ -236,7 +244,7 @@ func Test_gitlabTargetManager_Reconcile_success(t *testing.T) { glmock := gitlabmock.New( []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: time.Now(), }, }, @@ -257,7 +265,7 @@ func Test_gitlabTargetManager_Reconcile_success(t *testing.T) { }() time.Sleep(time.Millisecond * 300) - if gtm.GetTargets()[0].Url != "test" { + if gtm.GetTargets()[0].Url != "https://test" { t.Fatalf("Reconcile() did not receive the correct target") } if !gtm.Registered() { @@ -278,6 +286,7 @@ func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { tests := []struct { name string registered bool + targets []checks.GlobalTarget postErr error putError error }{ @@ -289,20 +298,19 @@ func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { name: "failure - failed to update registration", registered: true, putError: errors.New("failed to update registration"), - }, - } - - glmock := gitlabmock.New( - []checks.GlobalTarget{ - { - Url: "test", - LastSeen: time.Now(), + targets: []checks.GlobalTarget{ + { + Url: "https://test", + LastSeen: time.Now(), + }, }, }, - ) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + glmock := gitlabmock.New(tt.targets) + gtm := NewGitlabManager( glmock, "test", @@ -320,6 +328,7 @@ func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { time.Sleep(time.Millisecond * 300) + gtm.mu.Lock() if tt.postErr != nil && gtm.Registered() { t.Fatalf("Reconcile() should not have registered") } @@ -327,6 +336,7 @@ func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { if tt.putError != nil && !gtm.Registered() { t.Fatalf("Reconcile() should still be registered") } + gtm.mu.Unlock() err := gtm.Shutdown(ctx) if err != nil { @@ -342,7 +352,7 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { glmock := gitlabmock.New( []checks.GlobalTarget{ { - Url: "test", + Url: "https://test", LastSeen: time.Now(), }, }, @@ -365,8 +375,10 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { cancel() time.Sleep(time.Millisecond * 250) + gtm.mu.Lock() // instance shouldn't be registered anymore if gtm.Registered() { t.Fatalf("Reconcile() should not be registered") } + gtm.mu.Unlock() } From 72d4775dca752db608dd671b17dbc07e0812229d Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 15:27:11 +0100 Subject: [PATCH 13/28] chore: info -> debug Signed-off-by: Bruno Bressi --- pkg/sparrow/targets/gitlab.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index a535a0d8..ac8ee872 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -170,7 +170,7 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error { // filter unhealthy targets - this may be removed in the future for _, target := range targets { if !t.Registered() && target.Url == fmt.Sprintf("https://%s", t.name) { - log.Info("Found self as global target", "lastSeenMin", time.Since(target.LastSeen).Minutes()) + log.Debug("Found self as global target", "lastSeenMin", time.Since(target.LastSeen).Minutes()) t.registered = true } if time.Now().Add(-t.unhealthyThreshold).After(target.LastSeen) { From a71ae1bea554b310fb0eb37d34fa4ea171b6c622 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 15:35:17 +0100 Subject: [PATCH 14/28] chore: licencing Signed-off-by: Bruno Bressi --- cmd/gen-docs.go | 2 +- pkg/sparrow/gitlab/gitlab.go | 18 ++++++++++++++++++ pkg/sparrow/gitlab/gitlab_test.go | 18 ++++++++++++++++++ pkg/sparrow/gitlab/test/mockclient.go | 18 ++++++++++++++++++ pkg/sparrow/targets/gitlab.go | 18 ++++++++++++++++++ pkg/sparrow/targets/gitlab_test.go | 18 ++++++++++++++++++ pkg/sparrow/targets/targetmanager.go | 18 ++++++++++++++++++ 7 files changed, 109 insertions(+), 1 deletion(-) diff --git a/cmd/gen-docs.go b/cmd/gen-docs.go index bbb1bcdf..82ce54e7 100644 --- a/cmd/gen-docs.go +++ b/cmd/gen-docs.go @@ -25,7 +25,7 @@ import ( "github.com/spf13/cobra/doc" ) -// NewCmdRun creates a new gen-docs command +// NewCmdGenDocs creates a new gen-docs command func NewCmdGenDocs(rootCmd *cobra.Command) *cobra.Command { var docPath string diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 6b7f8300..5a533572 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package gitlab import ( diff --git a/pkg/sparrow/gitlab/gitlab_test.go b/pkg/sparrow/gitlab/gitlab_test.go index b2658590..21925d81 100644 --- a/pkg/sparrow/gitlab/gitlab_test.go +++ b/pkg/sparrow/gitlab/gitlab_test.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package gitlab import ( diff --git a/pkg/sparrow/gitlab/test/mockclient.go b/pkg/sparrow/gitlab/test/mockclient.go index 98790898..87f27c9f 100644 --- a/pkg/sparrow/gitlab/test/mockclient.go +++ b/pkg/sparrow/gitlab/test/mockclient.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package gitlabmock import ( diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index ac8ee872..e3f3d672 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package targets import ( diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index 0ad4b7de..2c0f465c 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package targets import ( diff --git a/pkg/sparrow/targets/targetmanager.go b/pkg/sparrow/targets/targetmanager.go index 77f37ec4..f9a3eacb 100644 --- a/pkg/sparrow/targets/targetmanager.go +++ b/pkg/sparrow/targets/targetmanager.go @@ -1,3 +1,21 @@ +// sparrow +// (C) 2023, Deutsche Telekom IT GmbH +// +// Deutsche Telekom IT GmbH and all other contributors / +// copyright owners license this file to you under the Apache +// License, Version 2.0 (the "License"); you may not use this +// file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package targets import ( From 7b29f0e5887eff88f34666d60d659d7b4d6ce1a5 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 16:01:04 +0100 Subject: [PATCH 15/28] docs: added target manager configuration Signed-off-by: Bruno Bressi --- README.md | 147 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 104 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index cbad09e4..f68854e9 100644 --- a/README.md +++ b/README.md @@ -8,34 +8,38 @@ - [About this component](#about-this-component) - [Installation](#installation) - - [Binary](#binary) - - [Container Image](#container-image) - - [Helm](#helm) + - [Binary](#binary) + - [Container Image](#container-image) + - [Helm](#helm) - [Usage](#usage) - - [Container Image](#container-image-1) + - [Container Image](#container-image-1) - [Configuration](#configuration) - - [Startup](#startup) - - [Loader](#loader) - - [Runtime](#runtime) - - [Check: Health](#check-health) - - [Check: Latency](#check-latency) - - [API](#api) + - [Startup](#startup) + - [Loader](#loader) + - [Runtime](#runtime) + - [TargetManager](#targetmanager) + - [Check: Health](#check-health) + - [Check: Latency](#check-latency) + - [API](#api) - [Code of Conduct](#code-of-conduct) - [Working Language](#working-language) - [Support and Feedback](#support-and-feedback) - [How to Contribute](#how-to-contribute) - [Licensing](#licensing) - -The `sparrow` is an infrastructure monitoring tool. The binary includes several checks (e.g. health check) that will be executed periodically. +The `sparrow` is an infrastructure monitoring tool. The binary includes several checks (e.g. health check) that will be +executed periodically. ## About this component -The `sparrow` performs several checks to monitor the health of the infrastructure and network from its point of view. The following checks are available: +The `sparrow` performs several checks to monitor the health of the infrastructure and network from its point of view. +The following checks are available: -1. Health check - `health`: The `sparrow` is able perform an http-based (HTTP/1.1) health check to provided endpoints. The `sparrow` will expose its own health check endpoint as well. +1. Health check - `health`: The `sparrow` is able perform an http-based (HTTP/1.1) health check to provided endpoints. + The `sparrow` will expose its own health check endpoint as well. -2. Latency check - `latency`: The `sparrow` is able to communicate with other `sparrow` instances to calculate the time a request takes to the target and back. The check is http (HTTP/1.1) based as well. +2. Latency check - `latency`: The `sparrow` is able to communicate with other `sparrow` instances to calculate the time + a request takes to the target and back. The check is http (HTTP/1.1) based as well. ## Installation @@ -45,7 +49,8 @@ Please see the [release notes](https://github.com/caas-team/sparrow/releases) fo ### Binary -The binary is available for several distributions. Currently the binary needs to be installed from a provided bundle or source. +The binary is available for several distributions. Currently the binary needs to be installed from a provided bundle or +source. ```sh curl https://github.com/caas-team/sparrow/releases/download/v${RELEASE_VERSION}/sparrow_${RELEASE_VERSION}_linux_amd64.tar.gz -Lo sparrow.tar.gz @@ -53,19 +58,22 @@ curl https://github.com/caas-team/sparrow/releases/download/v${RELEASE_VERSION}/ ``` For example release `v0.0.1`: + ```sh curl https://github.com/caas-team/sparrow/releases/download/v0.0.1/sparrow_0.0.1_linux_amd64.tar.gz -Lo sparrow.tar.gz curl https://github.com/caas-team/sparrow/releases/download/v0.0.1/sparrow_0.0.1_checksums.txt -Lo checksums.txt ``` Extract the binary: + ```sh tar -xf sparrow.tar.gz ``` ### Container Image -The [sparrow container images](https://github.com/caas-team/sparrow/pkgs/container/sparrow) for dedicated [release](https://github.com/caas-team/sparrow/releases) can be found in the GitHub registry. +The [sparrow container images](https://github.com/caas-team/sparrow/pkgs/container/sparrow) for +dedicated [release](https://github.com/caas-team/sparrow/releases) can be found in the GitHub registry. ### Helm @@ -75,7 +83,8 @@ Sparrow can be install via Helm Chart. The chart is provided in the GitHub regis helm -n sparrow upgrade -i sparrow oci://ghcr.io/caas-team/charts/sparrow --version 1.0.0 --create-namespace ``` -The default settings are fine for a local running configuration. With the default Helm values the sparrow loader uses a runtime configuration that is provided in a ConfigMap. The ConfigMap can be set by defining the `runtimeConfig` section. +The default settings are fine for a local running configuration. With the default Helm values the sparrow loader uses a +runtime configuration that is provided in a ConfigMap. The ConfigMap can be set by defining the `runtimeConfig` section. To be able to load the configuration during the runtime dynamically, the sparrow loader needs to be set to type `http`. @@ -86,8 +95,9 @@ startupConfig: loaderType: http loaderHttpUrl: https://url-to-runtime-config.de/api/config%2Eyaml -runtimeConfig: {} +runtimeConfig: { } ``` + For all available value options see [Chart README](./chart/README.md). Additionally check out the sparrow [configuration](#configuration) variants. @@ -102,11 +112,14 @@ Run a `sparrow` container by using e.g. `docker run ghcr.io/caas-team/sparrow`. Pass the available configuration arguments to the container e.g. `docker run ghcr.io/caas-team/sparrow --help`. -Start the instance using a mounted startup configuration file e.g. `docker run -v /config:/config ghcr.io/caas-team/sparrow --config /config/config.yaml`. +Start the instance using a mounted startup configuration file +e.g. `docker run -v /config:/config ghcr.io/caas-team/sparrow --config /config/config.yaml`. ## Configuration -The configuration is divided into two parts. The startup configuration and the runtime configuration. The startup configuration is a technical configuration to configure the `sparrow` instance itself. The runtime configuration will be loaded by the `loader` from a remote endpoint. This configuration consist of the checks configuration. +The configuration is divided into two parts. The startup configuration and the runtime configuration. The startup +configuration is a technical configuration to configure the `sparrow` instance itself. The runtime configuration will be +loaded by the `loader` from a remote endpoint. This configuration consist of the checks configuration. ### Startup @@ -117,7 +130,7 @@ The `sparrow` is able to get the startup configuration from different sources as Priority of configuration (high to low): 1. CLI flags -2. Environment variables +2. Environment variables 3. Defined configuration file 4. Default configuration file @@ -130,12 +143,18 @@ The loader can be selected by specifying the `loaderType` configuration paramete The default loader is an `http` loader that is able to get the runtime configuration from a remote endpoint. Available loader: -- `http`: The default. Loads configuration from a remote endpoint. Token authentication is available. Additional configuration parameter have the prefix `loaderHttp`. -- `file` (experimental): Loads configuration once from a local file. Additional configuration parameter have the prefix `loaderFile`. This is just for development purposes. + +- `http`: The default. Loads configuration from a remote endpoint. Token authentication is available. Additional + configuration parameter have the prefix `loaderHttp`. +- `file` (experimental): Loads configuration once from a local file. Additional configuration parameter have the + prefix `loaderFile`. This is just for development purposes. ### Runtime -Besides the technical startup configuration the configuration for the `sparrow` checks is loaded dynamically from an http endpoint. The `loader` is able to load the configuration dynamically during the runtime. Checks can be enabled, disabled and configured. The available loader confutation options for the startup configuration can be found in [here](sparrow_run.md) +Besides the technical startup configuration the configuration for the `sparrow` checks is loaded dynamically from an +http endpoint. The `loader` is able to load the configuration dynamically during the runtime. Checks can be enabled, +disabled and configured. The available loader confutation options for the startup configuration can be found +in [here](sparrow_run.md) Example format of a runtime configuration: @@ -147,13 +166,44 @@ checks: enabled: true ``` +### Target Manager + +The `sparrow` is able to manage the targets for the checks and register the `sparrow` as target on a (remote) backend. +This is done via a `TargetManager` interface, which can be configured on startup. The available configuration options +are listed below: + +| Type | Description | Default | +|--------------------------------------|--------------------------------------------------------------------------------------|----------------------| +| `targetManager.type` | The kind of target manager to use. | `gitlab` | +| `targetManager.checkInterval` | The interval in seconds to check for new targets. | `300` | +| `targetManager.unhealthyThreshold` | The threshold in seconds to mark a target as unhealthy and remove it from the state. | `600` | +| `targetManager.registrationInterval` | The interval in seconds to register the current sparrow at the targets backend. | `300` | +| `targetManager.gitlab.token` | The token to authenticate against the gitlab instance. | `""` | +| `targetManager.gitlab.url` | The base URL of the gitlab instance. | `https://gitlab.com` | +| `targetManager.gitlab.projectID` | The project ID of the gitlab project to use as a remote state backend. | `""` | + +The Gitlab target manager uses a gitlab project as the remote state backend. The various `sparrow` instances will +register themselves as targets in the project. The `sparrow` instances will also check the project for new targets and +add them to the local state. The registration is done by committing a "state" file in the main branch of the repository, +which is named after the DNS name of the `sparrow`. The state file contains the following information: + +```json +{ + "url": "https:// | ## How to Contribute -Contribution and feedback is encouraged and always welcome. For more information about how to contribute, the project structure, as well as additional contribution information, see our [Contribution Guidelines](./CONTRIBUTING.md). By participating in this project, you agree to abide by its [Code of Conduct](./CODE_OF_CONDUCT.md) at all times. +Contribution and feedback is encouraged and always welcome. For more information about how to contribute, the project +structure, as well as additional contribution information, see our [Contribution Guidelines](./CONTRIBUTING.md). By +participating in this project, you agree to abide by its [Code of Conduct](./CODE_OF_CONDUCT.md) at all times. ## Licensing Copyright (c) 2023 Deutsche Telekom IT GmbH. -Licensed under the **Apache License, Version 2.0** (the "License"); you may not use this file except in compliance with the License. +Licensed under the **Apache License, Version 2.0** (the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at . -Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the [LICENSE](./LICENSE) for the specific language governing permissions and limitations under the License. +Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an " +AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the [LICENSE](./LICENSE) for +the specific language governing permissions and limitations under the License. From 2a7fb027e9da32de3d06382091b1562485959cd4 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 17:23:46 +0100 Subject: [PATCH 16/28] feat: read target manager cfg from file Signed-off-by: Bruno Bressi --- README.md | 11 ++++---- cmd/run.go | 4 +++ docs/sparrow_run.md | 1 + go.mod | 1 + pkg/config/config.go | 45 ++++++++++++++++++++++++++++-- pkg/config/config_test.go | 26 +++++++++++++++++ pkg/config/flags.go | 2 ++ pkg/config/testdata/tmconfig.yaml | 7 +++++ pkg/sparrow/run.go | 28 +++---------------- pkg/sparrow/targets/gitlab.go | 12 ++++---- pkg/sparrow/targets/gitlab_test.go | 39 ++++++++++++-------------- 11 files changed, 118 insertions(+), 58 deletions(-) create mode 100644 pkg/config/config_test.go create mode 100644 pkg/config/testdata/tmconfig.yaml diff --git a/README.md b/README.md index f68854e9..4cc1a07f 100644 --- a/README.md +++ b/README.md @@ -170,19 +170,20 @@ checks: The `sparrow` is able to manage the targets for the checks and register the `sparrow` as target on a (remote) backend. This is done via a `TargetManager` interface, which can be configured on startup. The available configuration options -are listed below: +are listed below and can be set in a startup YAML configuration file (per default `tmconfig.yaml` in the current +directory). | Type | Description | Default | |--------------------------------------|--------------------------------------------------------------------------------------|----------------------| -| `targetManager.type` | The kind of target manager to use. | `gitlab` | | `targetManager.checkInterval` | The interval in seconds to check for new targets. | `300` | | `targetManager.unhealthyThreshold` | The threshold in seconds to mark a target as unhealthy and remove it from the state. | `600` | | `targetManager.registrationInterval` | The interval in seconds to register the current sparrow at the targets backend. | `300` | | `targetManager.gitlab.token` | The token to authenticate against the gitlab instance. | `""` | -| `targetManager.gitlab.url` | The base URL of the gitlab instance. | `https://gitlab.com` | -| `targetManager.gitlab.projectID` | The project ID of the gitlab project to use as a remote state backend. | `""` | +| `targetManager.gitlab.baseUrl` | The base URL of the gitlab instance. | `https://gitlab.com` | +| `targetManager.gitlab.projectId` | The project ID of the gitlab project to use as a remote state backend. | `""` | -The Gitlab target manager uses a gitlab project as the remote state backend. The various `sparrow` instances will +Currently, only one target manager exists: the Gitlab target manager. It uses a gitlab project as the remote state +backend. The various `sparrow` instances will register themselves as targets in the project. The `sparrow` instances will also check the project for new targets and add them to the local state. The registration is done by committing a "state" file in the main branch of the repository, which is named after the DNS name of the `sparrow`. The state file contains the following information: diff --git a/cmd/run.go b/cmd/run.go index aa330eea..8a185959 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -48,6 +48,7 @@ func NewCmdRun() *cobra.Command { LoaderHttpRetryCount: "loaderHttpRetryCount", LoaderHttpRetryDelay: "loaderHttpRetryDelay", LoaderFilePath: "loaderFilePath", + TargetManagerConfig: "tmconfig", } cmd := &cobra.Command{ @@ -67,6 +68,7 @@ func NewCmdRun() *cobra.Command { cmd.PersistentFlags().Int(flagMapping.LoaderHttpRetryCount, defaultHttpRetryCount, "http loader: Amount of retries trying to load the configuration") cmd.PersistentFlags().Int(flagMapping.LoaderHttpRetryDelay, defaultHttpRetryDelay, "http loader: The initial delay between retries in seconds") cmd.PersistentFlags().String(flagMapping.LoaderFilePath, "config.yaml", "file loader: The path to the file to read the runtime config from") + cmd.PersistentFlags().String(flagMapping.TargetManagerConfig, "tmconfig.yaml", "target manager: The path to the file to read the target manager config from") _ = viper.BindPFlag(flagMapping.ApiAddress, cmd.PersistentFlags().Lookup(flagMapping.ApiAddress)) _ = viper.BindPFlag(flagMapping.LoaderType, cmd.PersistentFlags().Lookup(flagMapping.LoaderType)) @@ -77,6 +79,7 @@ func NewCmdRun() *cobra.Command { _ = viper.BindPFlag(flagMapping.LoaderHttpRetryCount, cmd.PersistentFlags().Lookup(flagMapping.LoaderHttpRetryCount)) _ = viper.BindPFlag(flagMapping.LoaderHttpRetryDelay, cmd.PersistentFlags().Lookup(flagMapping.LoaderHttpRetryDelay)) _ = viper.BindPFlag(flagMapping.LoaderFilePath, cmd.PersistentFlags().Lookup(flagMapping.LoaderFilePath)) + _ = viper.BindPFlag(flagMapping.TargetManagerConfig, cmd.PersistentFlags().Lookup(flagMapping.TargetManagerConfig)) return cmd } @@ -88,6 +91,7 @@ func run(fm *config.RunFlagsNameMapping) func(cmd *cobra.Command, args []string) ctx := logger.IntoContext(context.Background(), log) cfg := config.NewConfig() + cfg.SetTargetManagerConfig(config.NewTargetManagerConfig(viper.GetString(fm.TargetManagerConfig))) cfg.SetApiAddress(viper.GetString(fm.ApiAddress)) diff --git a/docs/sparrow_run.md b/docs/sparrow_run.md index 52ac1f90..80b12f51 100644 --- a/docs/sparrow_run.md +++ b/docs/sparrow_run.md @@ -23,6 +23,7 @@ sparrow run [flags] --loaderHttpUrl string http loader: The url where to get the remote configuration --loaderInterval int defines the interval the loader reloads the configuration in seconds (default 300) -l, --loaderType string defines the loader type that will load the checks configuration during the runtime. The fallback is the fileLoader (default "http") + --tmconfig string target manager: The path to the file to read the target manager config from (default "tmconfig.yaml") ``` ### Options inherited from parent commands diff --git a/go.mod b/go.mod index 38bc5df6..1f04c586 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.21 require ( github.com/getkin/kin-openapi v0.120.0 github.com/go-chi/chi/v5 v5.0.10 + github.com/go-test/deep v1.0.8 github.com/jarcoal/httpmock v1.3.1 github.com/mitchellh/mapstructure v1.5.0 github.com/spf13/cobra v1.8.0 diff --git a/pkg/config/config.go b/pkg/config/config.go index 50f9fb15..a29f1f04 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -19,15 +19,32 @@ package config import ( + "os" "time" + "gopkg.in/yaml.v3" + "github.com/caas-team/sparrow/internal/helper" ) +type GitlabTargetManagerConfig struct { + BaseURL string `yaml:"baseUrl"` + Token string `yaml:"token"` + ProjectID int `yaml:"projectId"` +} + +type TargetManagerConfig struct { + CheckInterval time.Duration `yaml:"checkInterval"` + RegistrationInterval time.Duration `yaml:"registrationInterval"` + UnhealthyThreshold time.Duration `yaml:"unhealthyThreshold"` + Gitlab GitlabTargetManagerConfig `yaml:"gitlab"` +} + type Config struct { - Checks map[string]any - Loader LoaderConfig - Api ApiConfig + Checks map[string]any + Loader LoaderConfig + Api ApiConfig + TargetManager TargetManagerConfig } // ApiConfig is the configuration for the data API @@ -56,6 +73,23 @@ type FileLoaderConfig struct { path string } +// NewTargetManagerConfig creates a new TargetManagerConfig +// from the passed file +func NewTargetManagerConfig(path string) TargetManagerConfig { + var res TargetManagerConfig + f, err := os.ReadFile(path) + if err != nil { + panic("failed to read config file " + err.Error()) + } + + err = yaml.Unmarshal(f, &res) + if err != nil { + panic("failed to parse config file: " + err.Error()) + } + + return res +} + // NewConfig creates a new Config func NewConfig() *Config { return &Config{ @@ -108,3 +142,8 @@ func (c *Config) SetLoaderHttpRetryCount(retryCount int) { func (c *Config) SetLoaderHttpRetryDelay(retryDelay int) { c.Loader.http.retryCfg.Delay = time.Duration(retryDelay) * time.Second } + +// SetTargetManagerConfig sets the target manager config +func (c *Config) SetTargetManagerConfig(config TargetManagerConfig) { + c.TargetManager = config +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 00000000..3a453611 --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,26 @@ +package config + +import ( + "testing" + "time" + + "github.com/go-test/deep" +) + +func Test_NewTargetManagerConfig_Gitlab(t *testing.T) { + got := NewTargetManagerConfig("testdata/tmconfig.yaml") + want := TargetManagerConfig{ + CheckInterval: 300 * time.Second, + RegistrationInterval: 600 * time.Second, + UnhealthyThreshold: 900 * time.Second, + Gitlab: GitlabTargetManagerConfig{ + BaseURL: "https://gitlab.devops.telekom.de", + ProjectID: 666, + Token: "gitlab-token", + }, + } + + if diff := deep.Equal(got, want); diff != nil { + t.Error(diff) + } +} diff --git a/pkg/config/flags.go b/pkg/config/flags.go index 45eea6ac..098adc1b 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -29,4 +29,6 @@ type RunFlagsNameMapping struct { LoaderHttpRetryCount string LoaderHttpRetryDelay string LoaderFilePath string + + TargetManagerConfig string } diff --git a/pkg/config/testdata/tmconfig.yaml b/pkg/config/testdata/tmconfig.yaml new file mode 100644 index 00000000..9f615528 --- /dev/null +++ b/pkg/config/testdata/tmconfig.yaml @@ -0,0 +1,7 @@ +checkInterval: 300s +registrationInterval: 600s +unhealthyThreshold: 900s +gitlab: + token: gitlab-token + baseUrl: https://gitlab.devops.telekom.de + projectId: 666 diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 83eb4fbb..eecf616c 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -22,10 +22,7 @@ import ( "context" "fmt" "net/http" - "os" - "time" - "github.com/caas-team/sparrow/pkg/sparrow/gitlab" targets "github.com/caas-team/sparrow/pkg/sparrow/targets" "github.com/caas-team/sparrow/internal/logger" @@ -36,14 +33,6 @@ import ( "github.com/go-chi/chi/v5" ) -const ( - gitlabRegistrationProjectID = 237078 - globalTargetsCheckInterval = 5 * time.Minute - registrationUnhealthyThreshold = 15 * time.Minute - registrationInterval = 5 * time.Minute - gitlabBaseUrl = "https://gitlab.devops.telekom.de" -) - type Sparrow struct { db db.DB // the existing checks @@ -74,15 +63,12 @@ func New(cfg *config.Config) *Sparrow { cCfgChecks: make(chan map[string]any, 1), routingTree: api.NewRoutingTree(), router: chi.NewRouter(), - targets: targets.NewGitlabManager( - gitlab.New(gitlabBaseUrl, os.Getenv("GITLAB_TOKEN"), gitlabRegistrationProjectID), - "cool-sparrow.de", - globalTargetsCheckInterval, - registrationUnhealthyThreshold, - registrationInterval, - ), } + // Set the target manager + gm := targets.NewGitlabManager("sparrow-with-cfg-file", cfg.TargetManager) + sparrow.targets = gm + sparrow.loader = config.NewLoader(cfg, sparrow.cCfgChecks) sparrow.db = db.NewInMemory() return sparrow @@ -204,9 +190,3 @@ func fanInResults(checkChan chan checks.Result, cResult chan checks.ResultDTO, n } } } - -// GlobalTarget represents a GlobalTarget that can be checked -type GlobalTarget struct { - Url string `json:"url"` - LastSeen time.Time `json:"lastSeen"` -} diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index e3f3d672..63a6368f 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -24,6 +24,8 @@ import ( "sync" "time" + "github.com/caas-team/sparrow/pkg/config" + "github.com/caas-team/sparrow/pkg/checks" "github.com/caas-team/sparrow/pkg/sparrow/gitlab" @@ -52,13 +54,13 @@ type gitlabTargetManager struct { } // NewGitlabManager creates a new gitlabTargetManager -func NewGitlabManager(g gitlab.Gitlab, name string, checkInterval, unhealthyThreshold, regInterval time.Duration) *gitlabTargetManager { +func NewGitlabManager(name string, gtmConfig config.TargetManagerConfig) *gitlabTargetManager { return &gitlabTargetManager{ - gitlab: g, + gitlab: gitlab.New(gtmConfig.Gitlab.BaseURL, gtmConfig.Gitlab.Token, gtmConfig.Gitlab.ProjectID), name: name, - checkInterval: checkInterval, - registrationInterval: regInterval, - unhealthyThreshold: unhealthyThreshold, + checkInterval: gtmConfig.CheckInterval, + registrationInterval: gtmConfig.RegistrationInterval, + unhealthyThreshold: gtmConfig.UnhealthyThreshold, mu: sync.RWMutex{}, done: make(chan struct{}, 1), } diff --git a/pkg/sparrow/targets/gitlab_test.go b/pkg/sparrow/targets/gitlab_test.go index 2c0f465c..695644df 100644 --- a/pkg/sparrow/targets/gitlab_test.go +++ b/pkg/sparrow/targets/gitlab_test.go @@ -22,6 +22,7 @@ import ( "context" "errors" "fmt" + "sync" "testing" "time" @@ -270,13 +271,7 @@ func Test_gitlabTargetManager_Reconcile_success(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gtm := NewGitlabManager( - glmock, - "test", - time.Millisecond*100, - time.Hour*1, - time.Millisecond*150, - ) + gtm := mockGitlabTargetManager(glmock, "test") ctx := context.Background() go func() { gtm.Reconcile(ctx) @@ -329,13 +324,7 @@ func Test_gitlabTargetManager_Reconcile_failure(t *testing.T) { t.Run(tt.name, func(t *testing.T) { glmock := gitlabmock.New(tt.targets) - gtm := NewGitlabManager( - glmock, - "test", - time.Millisecond*100, - time.Hour*1, - time.Millisecond*150, - ) + gtm := mockGitlabTargetManager(glmock, "test") glmock.SetPostFileErr(tt.postErr) glmock.SetPutFileErr(tt.putError) @@ -376,13 +365,7 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { }, ) - gtm := NewGitlabManager( - glmock, - "test", - time.Millisecond*100, - time.Hour*1, - time.Millisecond*150, - ) + gtm := mockGitlabTargetManager(glmock, "test") ctx, cancel := context.WithCancel(context.Background()) go func() { @@ -400,3 +383,17 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) { } gtm.mu.Unlock() } + +func mockGitlabTargetManager(g *gitlabmock.MockClient, name string) *gitlabTargetManager { + return &gitlabTargetManager{ + targets: nil, + mu: sync.RWMutex{}, + done: make(chan struct{}, 1), + gitlab: g, + name: name, + checkInterval: 100 * time.Millisecond, + unhealthyThreshold: 1 * time.Second, + registrationInterval: 150 * time.Millisecond, + registered: false, + } +} From c9ca5483934f296e1b8467cd58871d383aff47d2 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 17:29:59 +0100 Subject: [PATCH 17/28] chore: added vs code testing opts Signed-off-by: Bruno Bressi --- .vscode/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 867e79be..d35d0a6b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,6 +1,7 @@ { "go.testFlags": [ "-race", - "-cover" + "-cover", + "-count=1" ] } \ No newline at end of file From 2d6366db33b94c64b167706cdb4f9028d8010cd6 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Fri, 15 Dec 2023 17:30:31 +0100 Subject: [PATCH 18/28] docs: typo fixed Signed-off-by: Bruno Bressi --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4cc1a07f..7d81f31d 100644 --- a/README.md +++ b/README.md @@ -190,7 +190,7 @@ which is named after the DNS name of the `sparrow`. The state file contains the ```json { - "url": "https://", "lastSeen": "2021-09-30T12:00:00Z" } ``` From 7594c82f9932fb1141fec7f5446a47bf0cfacd3d Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:08:37 +0100 Subject: [PATCH 19/28] chore: removed fullstop from logs Signed-off-by: Bruno Bressi --- pkg/sparrow/targets/gitlab.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sparrow/targets/gitlab.go b/pkg/sparrow/targets/gitlab.go index 63a6368f..666bf0f0 100644 --- a/pkg/sparrow/targets/gitlab.go +++ b/pkg/sparrow/targets/gitlab.go @@ -93,7 +93,7 @@ func (t *gitlabTargetManager) Reconcile(ctx context.Context) { } } case <-t.done: - log.Info("Ending Reconcile routine.") + log.Info("Ending Reconcile routine") return case <-checkTimer.C: err := t.refreshTargets(ctx) From 495150dd0ca1ade030de2ceded8fc8951cf5f602 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:12:18 +0100 Subject: [PATCH 20/28] chore: moved defer close before error return Signed-off-by: Bruno Bressi --- pkg/sparrow/gitlab/gitlab.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 5a533572..62115258 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -105,10 +105,6 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, log.Error("Failed to fetch file", "file", f, "error", err) return res, err } - if resp.StatusCode != http.StatusOK { - log.Error("Failed to fetch file", "status", resp.Status) - return res, fmt.Errorf("request failed, status is %s", resp.Status) - } defer func(Body io.ReadCloser) { err = Body.Close() @@ -117,6 +113,11 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, } }(resp.Body) + if resp.StatusCode != http.StatusOK { + log.Error("Failed to fetch file", "status", resp.Status) + return res, fmt.Errorf("request failed, status is %s", resp.Status) + } + err = json.NewDecoder(resp.Body).Decode(&res) if err != nil { log.Error("Failed to decode file after fetching", "file", f, "error", err) @@ -149,19 +150,26 @@ func (g *Client) fetchFileList(ctx context.Context) ([]string, error) { req.Header.Add("PRIVATE-TOKEN", g.token) req.Header.Add("Content-Type", "application/json") - res, err := g.client.Do(req) + resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer if err != nil { log.Error("Failed to fetch file list", "error", err) return nil, err } - if res.StatusCode != http.StatusOK { - log.Error("Failed to fetch file list", "status", res.Status) - return nil, fmt.Errorf("request failed, status is %s", res.Status) + + defer func(Body io.ReadCloser) { + err = Body.Close() + if err != nil { + log.Error("Failed to close response body", "error", err) + } + }(resp.Body) + + if resp.StatusCode != http.StatusOK { + log.Error("Failed to fetch file list", "status", resp.Status) + return nil, fmt.Errorf("request failed, status is %s", resp.Status) } - defer res.Body.Close() var fl []file - err = json.NewDecoder(res.Body).Decode(&fl) + err = json.NewDecoder(resp.Body).Decode(&fl) if err != nil { log.Error("Failed to decode file list", "error", err) return nil, err From 395c980e52ad8965261c92bafd78f31a1bfa19d9 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:14:20 +0100 Subject: [PATCH 21/28] chore: logger with file name Signed-off-by: Bruno Bressi --- pkg/sparrow/gitlab/gitlab.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/sparrow/gitlab/gitlab.go b/pkg/sparrow/gitlab/gitlab.go index 62115258..e65f5c29 100644 --- a/pkg/sparrow/gitlab/gitlab.go +++ b/pkg/sparrow/gitlab/gitlab.go @@ -84,7 +84,7 @@ func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) // fetchFile fetches the file from the global targets repository from the configured gitlab repository func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, error) { - log := logger.FromContext(ctx) + log := logger.FromContext(ctx).With("file", f) var res checks.GlobalTarget // URL encode the name n := url.PathEscape(f) @@ -102,7 +102,7 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, resp, err := g.client.Do(req) //nolint:bodyclose // closed in defer if err != nil { - log.Error("Failed to fetch file", "file", f, "error", err) + log.Error("Failed to fetch file", "error", err) return res, err } @@ -120,11 +120,11 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget, err = json.NewDecoder(resp.Body).Decode(&res) if err != nil { - log.Error("Failed to decode file after fetching", "file", f, "error", err) + log.Error("Failed to decode file after fetching", "error", err) return res, err } - log.Debug("Successfully fetched file", "file", f) + log.Debug("Successfully fetched file") return res, nil } From f8cdaa6d389ee315cc59a83a1909c917817bea65 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:25:33 +0100 Subject: [PATCH 22/28] chore: ignore test_sast config Signed-off-by: Bruno Bressi --- .github/workflows/gosec-conf.json | 6 ++++++ .github/workflows/test_sast.yml | 2 +- pkg/config/config.go | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/gosec-conf.json diff --git a/.github/workflows/gosec-conf.json b/.github/workflows/gosec-conf.json new file mode 100644 index 00000000..b3bfb1e0 --- /dev/null +++ b/.github/workflows/gosec-conf.json @@ -0,0 +1,6 @@ +{ + "global": { + "nosec": "enabled", + "audit": "false" + } +} \ No newline at end of file diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index dacd396b..44f36aba 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -20,4 +20,4 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: ./... \ No newline at end of file + args: -config gosec-conf.json ./... \ No newline at end of file diff --git a/pkg/config/config.go b/pkg/config/config.go index a29f1f04..2b545690 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -77,7 +77,7 @@ type FileLoaderConfig struct { // from the passed file func NewTargetManagerConfig(path string) TargetManagerConfig { var res TargetManagerConfig - f, err := os.ReadFile(path) + f, err := os.ReadFile(path) //#nosec if err != nil { panic("failed to read config file " + err.Error()) } From efce1a096a183a8e1122495c303246a7ff25d14e Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:26:21 +0100 Subject: [PATCH 23/28] chore: added gosec on each push Signed-off-by: Bruno Bressi --- .github/workflows/test_sast.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index 44f36aba..7104daca 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -1,6 +1,7 @@ name: Test - SAST on: + push: pull_request: permissions: From c556271b6f895a1fa15c1645c4bb2be82d41902f Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:27:37 +0100 Subject: [PATCH 24/28] chore: gosec conf Signed-off-by: Bruno Bressi --- .github/workflows/test_sast.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index 7104daca..588b0c63 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -21,4 +21,4 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: -config gosec-conf.json ./... \ No newline at end of file + args: -conf gosec-conf.json ./... \ No newline at end of file From 69f39745c8f605d29ef3d2f14feddeb8f6bf76c2 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:28:58 +0100 Subject: [PATCH 25/28] debug: possibly fixed config path Signed-off-by: Bruno Bressi --- .github/workflows/test_sast.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index 588b0c63..1fb47f98 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -21,4 +21,4 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: -conf gosec-conf.json ./... \ No newline at end of file + args: -conf ./github/workflows/gosec-conf.json ./... \ No newline at end of file From 098730282f3230220949f58f6cc9bc3bf3c764e1 Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:31:10 +0100 Subject: [PATCH 26/28] fix: folder renamed to .github Signed-off-by: Bruno Bressi --- .github/workflows/test_sast.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index 1fb47f98..fcc50587 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -21,4 +21,4 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: -conf ./github/workflows/gosec-conf.json ./... \ No newline at end of file + args: -conf ./.github/workflows/gosec-conf.json ./... \ No newline at end of file From c3f252bb3c844a8077f1e4a4d7ffc561819f434e Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:35:56 +0100 Subject: [PATCH 27/28] chore: ignore nosec for the target manager loading Signed-off-by: Bruno Bressi --- pkg/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 2b545690..58503bdf 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -77,7 +77,7 @@ type FileLoaderConfig struct { // from the passed file func NewTargetManagerConfig(path string) TargetManagerConfig { var res TargetManagerConfig - f, err := os.ReadFile(path) //#nosec + f, err := os.ReadFile(path) // #nosec G304 if err != nil { panic("failed to read config file " + err.Error()) } From fa30913e2938eb3cebef442ac282c076aa6f297a Mon Sep 17 00:00:00 2001 From: Bruno Bressi Date: Mon, 18 Dec 2023 16:42:51 +0100 Subject: [PATCH 28/28] fix: removed gosec config Signed-off-by: Bruno Bressi --- .github/workflows/gosec-conf.json | 6 ------ .github/workflows/test_sast.yml | 2 +- pkg/config/config.go | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) delete mode 100644 .github/workflows/gosec-conf.json diff --git a/.github/workflows/gosec-conf.json b/.github/workflows/gosec-conf.json deleted file mode 100644 index b3bfb1e0..00000000 --- a/.github/workflows/gosec-conf.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "global": { - "nosec": "enabled", - "audit": "false" - } -} \ No newline at end of file diff --git a/.github/workflows/test_sast.yml b/.github/workflows/test_sast.yml index fcc50587..2ed4eeba 100644 --- a/.github/workflows/test_sast.yml +++ b/.github/workflows/test_sast.yml @@ -21,4 +21,4 @@ jobs: - name: Run Gosec Security Scanner uses: securego/gosec@master with: - args: -conf ./.github/workflows/gosec-conf.json ./... \ No newline at end of file + args: ./... \ No newline at end of file diff --git a/pkg/config/config.go b/pkg/config/config.go index 58503bdf..4f57b84b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -77,7 +77,7 @@ type FileLoaderConfig struct { // from the passed file func NewTargetManagerConfig(path string) TargetManagerConfig { var res TargetManagerConfig - f, err := os.ReadFile(path) // #nosec G304 + f, err := os.ReadFile(path) //#nosec G304 if err != nil { panic("failed to read config file " + err.Error()) }