diff --git a/apiserver/controllers/controllers.go b/apiserver/controllers/controllers.go index fce100a6..e3495e9c 100644 --- a/apiserver/controllers/controllers.go +++ b/apiserver/controllers/controllers.go @@ -114,7 +114,7 @@ func (a *APIController) handleWorkflowJobEvent(ctx context.Context, w http.Respo ).Inc() slog.With(slog.Any("error", err)).ErrorContext(ctx, "got not found error from DispatchWorkflowJob. webhook not meant for us?") return - } else if strings.Contains(err.Error(), "signature") { // TODO: check error type + } else if strings.Contains(err.Error(), "signature") { // nolint:golangci-lint,godox TODO: check error type metrics.WebhooksReceived.WithLabelValues( "false", // label: valid "signature_invalid", // label: reason @@ -182,6 +182,7 @@ func (a *APIController) WSHandler(writer http.ResponseWriter, req *http.Request) return } + // nolint:golangci-lint,godox // TODO (gsamfira): Handle ExpiresAt. Right now, if a client uses // a valid token to authenticate, and keeps the websocket connection // open, it will allow that client to stream logs via websockets diff --git a/auth/auth.go b/auth/auth.go index 6b782bee..4a4f957a 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -58,6 +58,7 @@ func (a *Authenticator) GetJWTToken(ctx context.Context) (string, error) { claims := JWTClaims{ RegisteredClaims: jwt.RegisteredClaims{ ExpiresAt: expires, + // nolint:golangci-lint,godox // TODO: make this configurable Issuer: "garm", }, @@ -86,6 +87,7 @@ func (a *Authenticator) GetJWTMetricsToken(ctx context.Context) (string, error) if err != nil { return "", errors.Wrap(err, "generating random string") } + // nolint:golangci-lint,godox // TODO: currently this is the same TTL as the normal Token // maybe we should make this configurable // it's usually pretty nasty if the monitoring fails because the token expired @@ -96,6 +98,7 @@ func (a *Authenticator) GetJWTMetricsToken(ctx context.Context) (string, error) claims := JWTClaims{ RegisteredClaims: jwt.RegisteredClaims{ ExpiresAt: expires, + // nolint:golangci-lint,godox // TODO: make this configurable Issuer: "garm", }, diff --git a/auth/instance_middleware.go b/auth/instance_middleware.go index 7d5528fe..54fca249 100644 --- a/auth/instance_middleware.go +++ b/auth/instance_middleware.go @@ -111,6 +111,7 @@ func (amw *instanceMiddleware) claimsToContext(ctx context.Context, claims *Inst // Middleware implements the middleware interface func (amw *instanceMiddleware) Middleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // nolint:golangci-lint,godox // TODO: Log error details when authentication fails ctx := r.Context() authorizationHeader := r.Header.Get("authorization") diff --git a/auth/jwt.go b/auth/jwt.go index 15909d75..d463df2c 100644 --- a/auth/jwt.go +++ b/auth/jwt.go @@ -87,6 +87,7 @@ func invalidAuthResponse(ctx context.Context, w http.ResponseWriter) { // Middleware implements the middleware interface func (amw *jwtMiddleware) Middleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // nolint:golangci-lint,godox // TODO: Log error details when authentication fails ctx := r.Context() authorizationHeader := r.Header.Get("authorization") diff --git a/config/config.go b/config/config.go index 84084be1..bea1b5b8 100644 --- a/config/config.go +++ b/config/config.go @@ -546,6 +546,7 @@ func (d *timeToLive) Duration() time.Duration { slog.With(slog.Any("error", err)).Error("failed to parse duration") return appdefaults.DefaultJWTTTL } + // nolint:golangci-lint,godox // TODO(gabriel-samfira): should we have a minimum TTL? if duration < appdefaults.DefaultJWTTTL { return appdefaults.DefaultJWTTTL diff --git a/database/common/common.go b/database/common/common.go index d059245c..6f8dc820 100644 --- a/database/common/common.go +++ b/database/common/common.go @@ -77,6 +77,7 @@ type EnterpriseStore interface { type PoolStore interface { // Probably a bad idea without some king of filter or at least pagination + // nolint:golangci-lint,godox // TODO: add filter/pagination ListAllPools(ctx context.Context) ([]params.Pool, error) GetPoolByID(ctx context.Context, poolID string) (params.Pool, error) @@ -104,6 +105,8 @@ type InstanceStore interface { UpdateInstance(ctx context.Context, instanceID string, param params.UpdateInstanceParams) (params.Instance, error) // Probably a bad idea without some king of filter or at least pagination + // + // nolint:golangci-lint,godox // TODO: add filter/pagination ListAllInstances(ctx context.Context) ([]params.Instance, error) diff --git a/runner/pool/enterprise.go b/runner/pool/enterprise.go index e389ea52..5e0b1e7e 100644 --- a/runner/pool/enterprise.go +++ b/runner/pool/enterprise.go @@ -78,6 +78,7 @@ type enterprise struct { } func (e *enterprise) findRunnerGroupByName(name string) (*github.EnterpriseRunnerGroup, error) { + // nolint:golangci-lint,godox // TODO(gabriel-samfira): implement caching opts := github.ListEnterpriseRunnerGroupOptions{ ListOptions: github.ListOptions{ @@ -129,6 +130,7 @@ func (e *enterprise) GetJITConfig(ctx context.Context, instance string, pool par Name: instance, RunnerGroupID: rg, Labels: labels, + // nolint:golangci-lint,godox // TODO(gabriel-samfira): Should we make this configurable? WorkFolder: github.String("_work"), } diff --git a/runner/pool/organization.go b/runner/pool/organization.go index f3598866..aa434141 100644 --- a/runner/pool/organization.go +++ b/runner/pool/organization.go @@ -90,6 +90,7 @@ type organization struct { } func (o *organization) findRunnerGroupByName(name string) (*github.RunnerGroup, error) { + // nolint:golangci-lint,godox // TODO(gabriel-samfira): implement caching opts := github.ListOrgRunnerGroupOptions{ ListOptions: github.ListOptions{ @@ -141,6 +142,7 @@ func (o *organization) GetJITConfig(ctx context.Context, instance string, pool p Name: instance, RunnerGroupID: rg, Labels: labels, + // nolint:golangci-lint,godox // TODO(gabriel-samfira): Should we make this configurable? WorkFolder: github.String("_work"), } diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 2ecfcece..84f2258f 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -54,6 +54,8 @@ var ( const ( // maxCreateAttempts is the number of times we will attempt to create an instance // before we give up. + // + // nolint:golangci-lint,godox // TODO: make this configurable(?) maxCreateAttempts = 5 @@ -1143,6 +1145,7 @@ func (r *basePoolManager) scaleDownOnePool(ctx context.Context, pool params.Pool // up by a runner, they are most likely stale and can be removed. For now, we can simply // remove jobs older than 10 minutes. // + // nolint:golangci-lint,godox // TODO: should probably allow aditional filters to list functions. Would help to filter by date // instead of returning a bunch of results and filtering manually. queued, err := r.store.ListEntityJobsByStatus(r.ctx, r.helper.PoolType(), r.helper.ID(), params.JobStatusQueued) @@ -1276,6 +1279,7 @@ func (r *basePoolManager) retryFailedInstancesForOnePool(ctx context.Context, po slog.DebugContext( ctx, "attempting to clean up any previous instance", "runner_name", instance.Name) + // nolint:golangci-lint,godox // NOTE(gabriel-samfira): this is done in parallel. If there are many failed instances // this has the potential to create many API requests to the target provider. // TODO(gabriel-samfira): implement request throttling. @@ -1295,6 +1299,7 @@ func (r *basePoolManager) retryFailedInstancesForOnePool(ctx context.Context, po slog.DebugContext( ctx, "cleanup of previously failed instance complete", "runner_name", instance.Name) + // nolint:golangci-lint,godox // TODO(gabriel-samfira): Incrementing CreateAttempt should be done within a transaction. // It's fairly safe to do here (for now), as there should be no other code path that updates // an instance in this state. @@ -1504,6 +1509,7 @@ func (r *basePoolManager) deletePendingInstances() error { } func (r *basePoolManager) addPendingInstances() error { + // nolint:golangci-lint,godox // TODO: filter instances by status. instances, err := r.helper.FetchDbInstances() if err != nil { @@ -1770,6 +1776,7 @@ func (r *basePoolManager) consumeQueuedJobs() error { // was spawned. Unlock it and try again. A different job may have picked up // the runner. if err := r.store.UnlockJob(r.ctx, job.ID, r.ID()); err != nil { + // nolint:golangci-lint,godox // TODO: Implament a cache? Should we return here? slog.With(slog.Any("error", err)).ErrorContext( r.ctx, "failed to unlock job", @@ -1779,6 +1786,7 @@ func (r *basePoolManager) consumeQueuedJobs() error { } if job.LockedBy.String() == r.ID() { + // nolint:golangci-lint,godox // Job is locked by us. We must have already attepted to create a runner for it. Skip. // TODO(gabriel-samfira): create an in-memory state of existing runners that we can easily // check for existing pending or idle runners. If we can't find any, attempt to allocate another diff --git a/runner/pool/repository.go b/runner/pool/repository.go index 856995ad..14e80319 100644 --- a/runner/pool/repository.go +++ b/runner/pool/repository.go @@ -99,6 +99,7 @@ func (r *repository) GetJITConfig(ctx context.Context, instance string, pool par // At the repository level we only have the default runner group. RunnerGroupID: 1, Labels: labels, + // nolint:golangci-lint,godox // TODO(gabriel-samfira): Should we make this configurable? WorkFolder: github.String("_work"), } diff --git a/runner/providers/external/external.go b/runner/providers/external/external.go index 52c672b7..446f69a5 100644 --- a/runner/providers/external/external.go +++ b/runner/providers/external/external.go @@ -157,6 +157,7 @@ func (e *external) GetInstance(ctx context.Context, instance string) (commonPara } asEnv = append(asEnv, e.environmentVariables...) + // nolint:golangci-lint,godox // TODO(gabriel-samfira): handle error types. Of particular interest is to // know when the error is ErrNotFound. metrics.InstanceOperationCount.WithLabelValues(