fix: godoc linter warnings (TODOs)

Signed-off-by: Mario Constanti <mario.constanti@mercedes-benz.com>
This commit is contained in:
Mario Constanti 2024-02-22 09:30:20 +01:00
parent acc17eafcd
commit b0e3f78fbb
11 changed files with 25 additions and 1 deletions

View file

@ -114,7 +114,7 @@ func (a *APIController) handleWorkflowJobEvent(ctx context.Context, w http.Respo
).Inc() ).Inc()
slog.With(slog.Any("error", err)).ErrorContext(ctx, "got not found error from DispatchWorkflowJob. webhook not meant for us?") slog.With(slog.Any("error", err)).ErrorContext(ctx, "got not found error from DispatchWorkflowJob. webhook not meant for us?")
return 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( metrics.WebhooksReceived.WithLabelValues(
"false", // label: valid "false", // label: valid
"signature_invalid", // label: reason "signature_invalid", // label: reason
@ -182,6 +182,7 @@ func (a *APIController) WSHandler(writer http.ResponseWriter, req *http.Request)
return return
} }
// nolint:golangci-lint,godox
// TODO (gsamfira): Handle ExpiresAt. Right now, if a client uses // TODO (gsamfira): Handle ExpiresAt. Right now, if a client uses
// a valid token to authenticate, and keeps the websocket connection // a valid token to authenticate, and keeps the websocket connection
// open, it will allow that client to stream logs via websockets // open, it will allow that client to stream logs via websockets

View file

@ -58,6 +58,7 @@ func (a *Authenticator) GetJWTToken(ctx context.Context) (string, error) {
claims := JWTClaims{ claims := JWTClaims{
RegisteredClaims: jwt.RegisteredClaims{ RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: expires, ExpiresAt: expires,
// nolint:golangci-lint,godox
// TODO: make this configurable // TODO: make this configurable
Issuer: "garm", Issuer: "garm",
}, },
@ -86,6 +87,7 @@ func (a *Authenticator) GetJWTMetricsToken(ctx context.Context) (string, error)
if err != nil { if err != nil {
return "", errors.Wrap(err, "generating random string") return "", errors.Wrap(err, "generating random string")
} }
// nolint:golangci-lint,godox
// TODO: currently this is the same TTL as the normal Token // TODO: currently this is the same TTL as the normal Token
// maybe we should make this configurable // maybe we should make this configurable
// it's usually pretty nasty if the monitoring fails because the token expired // 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{ claims := JWTClaims{
RegisteredClaims: jwt.RegisteredClaims{ RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: expires, ExpiresAt: expires,
// nolint:golangci-lint,godox
// TODO: make this configurable // TODO: make this configurable
Issuer: "garm", Issuer: "garm",
}, },

View file

@ -111,6 +111,7 @@ func (amw *instanceMiddleware) claimsToContext(ctx context.Context, claims *Inst
// Middleware implements the middleware interface // Middleware implements the middleware interface
func (amw *instanceMiddleware) Middleware(next http.Handler) http.Handler { func (amw *instanceMiddleware) Middleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// nolint:golangci-lint,godox
// TODO: Log error details when authentication fails // TODO: Log error details when authentication fails
ctx := r.Context() ctx := r.Context()
authorizationHeader := r.Header.Get("authorization") authorizationHeader := r.Header.Get("authorization")

View file

@ -87,6 +87,7 @@ func invalidAuthResponse(ctx context.Context, w http.ResponseWriter) {
// Middleware implements the middleware interface // Middleware implements the middleware interface
func (amw *jwtMiddleware) Middleware(next http.Handler) http.Handler { func (amw *jwtMiddleware) Middleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// nolint:golangci-lint,godox
// TODO: Log error details when authentication fails // TODO: Log error details when authentication fails
ctx := r.Context() ctx := r.Context()
authorizationHeader := r.Header.Get("authorization") authorizationHeader := r.Header.Get("authorization")

View file

@ -546,6 +546,7 @@ func (d *timeToLive) Duration() time.Duration {
slog.With(slog.Any("error", err)).Error("failed to parse duration") slog.With(slog.Any("error", err)).Error("failed to parse duration")
return appdefaults.DefaultJWTTTL return appdefaults.DefaultJWTTTL
} }
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): should we have a minimum TTL? // TODO(gabriel-samfira): should we have a minimum TTL?
if duration < appdefaults.DefaultJWTTTL { if duration < appdefaults.DefaultJWTTTL {
return appdefaults.DefaultJWTTTL return appdefaults.DefaultJWTTTL

View file

@ -77,6 +77,7 @@ type EnterpriseStore interface {
type PoolStore interface { type PoolStore interface {
// Probably a bad idea without some king of filter or at least pagination // Probably a bad idea without some king of filter or at least pagination
// nolint:golangci-lint,godox
// TODO: add filter/pagination // TODO: add filter/pagination
ListAllPools(ctx context.Context) ([]params.Pool, error) ListAllPools(ctx context.Context) ([]params.Pool, error)
GetPoolByID(ctx context.Context, poolID string) (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) 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 // Probably a bad idea without some king of filter or at least pagination
//
// nolint:golangci-lint,godox
// TODO: add filter/pagination // TODO: add filter/pagination
ListAllInstances(ctx context.Context) ([]params.Instance, error) ListAllInstances(ctx context.Context) ([]params.Instance, error)

View file

@ -78,6 +78,7 @@ type enterprise struct {
} }
func (e *enterprise) findRunnerGroupByName(name string) (*github.EnterpriseRunnerGroup, error) { func (e *enterprise) findRunnerGroupByName(name string) (*github.EnterpriseRunnerGroup, error) {
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): implement caching // TODO(gabriel-samfira): implement caching
opts := github.ListEnterpriseRunnerGroupOptions{ opts := github.ListEnterpriseRunnerGroupOptions{
ListOptions: github.ListOptions{ ListOptions: github.ListOptions{
@ -129,6 +130,7 @@ func (e *enterprise) GetJITConfig(ctx context.Context, instance string, pool par
Name: instance, Name: instance,
RunnerGroupID: rg, RunnerGroupID: rg,
Labels: labels, Labels: labels,
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): Should we make this configurable? // TODO(gabriel-samfira): Should we make this configurable?
WorkFolder: github.String("_work"), WorkFolder: github.String("_work"),
} }

View file

@ -90,6 +90,7 @@ type organization struct {
} }
func (o *organization) findRunnerGroupByName(name string) (*github.RunnerGroup, error) { func (o *organization) findRunnerGroupByName(name string) (*github.RunnerGroup, error) {
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): implement caching // TODO(gabriel-samfira): implement caching
opts := github.ListOrgRunnerGroupOptions{ opts := github.ListOrgRunnerGroupOptions{
ListOptions: github.ListOptions{ ListOptions: github.ListOptions{
@ -141,6 +142,7 @@ func (o *organization) GetJITConfig(ctx context.Context, instance string, pool p
Name: instance, Name: instance,
RunnerGroupID: rg, RunnerGroupID: rg,
Labels: labels, Labels: labels,
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): Should we make this configurable? // TODO(gabriel-samfira): Should we make this configurable?
WorkFolder: github.String("_work"), WorkFolder: github.String("_work"),
} }

View file

@ -54,6 +54,8 @@ var (
const ( const (
// maxCreateAttempts is the number of times we will attempt to create an instance // maxCreateAttempts is the number of times we will attempt to create an instance
// before we give up. // before we give up.
//
// nolint:golangci-lint,godox
// TODO: make this configurable(?) // TODO: make this configurable(?)
maxCreateAttempts = 5 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 // up by a runner, they are most likely stale and can be removed. For now, we can simply
// remove jobs older than 10 minutes. // 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 // 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. // 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) 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( slog.DebugContext(
ctx, "attempting to clean up any previous instance", ctx, "attempting to clean up any previous instance",
"runner_name", instance.Name) "runner_name", instance.Name)
// nolint:golangci-lint,godox
// NOTE(gabriel-samfira): this is done in parallel. If there are many failed instances // 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. // this has the potential to create many API requests to the target provider.
// TODO(gabriel-samfira): implement request throttling. // TODO(gabriel-samfira): implement request throttling.
@ -1295,6 +1299,7 @@ func (r *basePoolManager) retryFailedInstancesForOnePool(ctx context.Context, po
slog.DebugContext( slog.DebugContext(
ctx, "cleanup of previously failed instance complete", ctx, "cleanup of previously failed instance complete",
"runner_name", instance.Name) "runner_name", instance.Name)
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): Incrementing CreateAttempt should be done within a transaction. // 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 // It's fairly safe to do here (for now), as there should be no other code path that updates
// an instance in this state. // an instance in this state.
@ -1504,6 +1509,7 @@ func (r *basePoolManager) deletePendingInstances() error {
} }
func (r *basePoolManager) addPendingInstances() error { func (r *basePoolManager) addPendingInstances() error {
// nolint:golangci-lint,godox
// TODO: filter instances by status. // TODO: filter instances by status.
instances, err := r.helper.FetchDbInstances() instances, err := r.helper.FetchDbInstances()
if err != nil { 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 // was spawned. Unlock it and try again. A different job may have picked up
// the runner. // the runner.
if err := r.store.UnlockJob(r.ctx, job.ID, r.ID()); err != nil { 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? // TODO: Implament a cache? Should we return here?
slog.With(slog.Any("error", err)).ErrorContext( slog.With(slog.Any("error", err)).ErrorContext(
r.ctx, "failed to unlock job", r.ctx, "failed to unlock job",
@ -1779,6 +1786,7 @@ func (r *basePoolManager) consumeQueuedJobs() error {
} }
if job.LockedBy.String() == r.ID() { 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. // 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 // 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 // check for existing pending or idle runners. If we can't find any, attempt to allocate another

View file

@ -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. // At the repository level we only have the default runner group.
RunnerGroupID: 1, RunnerGroupID: 1,
Labels: labels, Labels: labels,
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): Should we make this configurable? // TODO(gabriel-samfira): Should we make this configurable?
WorkFolder: github.String("_work"), WorkFolder: github.String("_work"),
} }

View file

@ -157,6 +157,7 @@ func (e *external) GetInstance(ctx context.Context, instance string) (commonPara
} }
asEnv = append(asEnv, e.environmentVariables...) asEnv = append(asEnv, e.environmentVariables...)
// nolint:golangci-lint,godox
// TODO(gabriel-samfira): handle error types. Of particular interest is to // TODO(gabriel-samfira): handle error types. Of particular interest is to
// know when the error is ErrNotFound. // know when the error is ErrNotFound.
metrics.InstanceOperationCount.WithLabelValues( metrics.InstanceOperationCount.WithLabelValues(