Skip to content

Commit

Permalink
Fix gitauth secrets cleanup
Browse files Browse the repository at this point in the history
The gitauth secrets are created before the pipelineRun and deleted
through ownerRef when the pipelineRun is deleted. This fixes the issue
where the secrets are left in the namespace if the pipelineRun creation
fails, hitting the secrets quota and blocking subsequent pipelineRuns.
  • Loading branch information
enarha authored and chmouel committed Nov 6, 2024
1 parent 3e8567c commit 3723cde
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
1 change: 1 addition & 0 deletions pkg/kubeinteraction/kubeinteraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
type Interface interface {
CleanupPipelines(context.Context, *zap.SugaredLogger, *v1alpha1.Repository, *pipelinev1.PipelineRun, int) error
CreateSecret(ctx context.Context, ns string, secret *corev1.Secret) error
DeleteSecret(context.Context, *zap.SugaredLogger, string, string) error
UpdateSecretWithOwnerRef(context.Context, *zap.SugaredLogger, string, string, *pipelinev1.PipelineRun) error
GetSecret(context.Context, ktypes.GetSecretOpt) (string, error)
GetPodLogs(context.Context, string, string, string, int64) (string, error)
Expand Down
28 changes: 18 additions & 10 deletions pkg/pipelineascode/pipelineascode.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,32 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
match.PipelineRun.Annotations[keys.State] = kubeinteraction.StateQueued
}

// Create the actual pipeline
// Create the actual pipelineRun
pr, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.GetNamespace()).Create(ctx,
match.PipelineRun, metav1.CreateOptions{})
if err != nil {
// cleanup the gitauth secret because ownerRef isn't set when the pipelineRun creation failed
if p.pacInfo.SecretAutoCreation {
if errDelSec := p.k8int.DeleteSecret(ctx, p.logger, match.Repo.GetNamespace(), gitAuthSecretName); errDelSec != nil {
// don't overshadow the pipelineRun creation error, just log
p.logger.Errorf("removing auto created secret: %s in namespace %s has failed: %w ", gitAuthSecretName, match.Repo.GetNamespace(), errDelSec)
}
}
// we need to make difference between markdown error and normal error that goes to namespace/controller stream
return nil, fmt.Errorf("creating pipelinerun %s in namespace %s has failed.\n\nTekton Controller has reported this error: ```%w``` ", match.PipelineRun.GetGenerateName(),
match.Repo.GetNamespace(), err)
}

// update ownerRef of secret with pipelineRun, so that it gets cleanedUp with pipelineRun
if p.pacInfo.SecretAutoCreation {
err := p.k8int.UpdateSecretWithOwnerRef(ctx, p.logger, pr.Namespace, gitAuthSecretName, pr)
if err != nil {
// we still return the created PR with error, and allow caller to decide what to do with the PR, and avoid
// unneeded SIGSEGV's
return pr, fmt.Errorf("cannot update pipelinerun %s with ownerRef: %w", pr.GetGenerateName(), err)
}
}

// Create status with the log url
p.logger.Infof("pipelinerun %s has been created in namespace %s for SHA: %s Target Branch: %s",
pr.GetName(), match.Repo.GetNamespace(), p.event.SHA, p.event.BaseBranch)
Expand Down Expand Up @@ -236,15 +253,6 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
}
}

// update ownerRef of secret with pipelineRun, so that it gets cleanedUp with pipelineRun
if p.pacInfo.SecretAutoCreation {
err := p.k8int.UpdateSecretWithOwnerRef(ctx, p.logger, pr.Namespace, gitAuthSecretName, pr)
if err != nil {
// we still return the created PR with error, and allow caller to decide what to do with the PR, and avoid
// unneeded SIGSEGV's
return pr, fmt.Errorf("cannot update pipelinerun %s with ownerRef: %w", pr.GetGenerateName(), err)
}
}
return pr, nil
}

Expand Down

0 comments on commit 3723cde

Please sign in to comment.