-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Auto configure new repo (only with GH app) #890
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -106,8 +106,6 @@ func (l listener) handleEvent() http.HandlerFunc { | |||
return | ||||
} | ||||
|
||||
// read request Body | ||||
|
||||
// event body | ||||
payload, err := io.ReadAll(request.Body) | ||||
if err != nil { | ||||
|
@@ -128,6 +126,25 @@ func (l listener) handleEvent() http.HandlerFunc { | |||
var logger *zap.SugaredLogger | ||||
|
||||
l.event = info.NewEvent() | ||||
|
||||
// if repository auto configuration is enabled then check if its a valid event | ||||
if l.run.Info.Pac.AutoConfigureNewRepo { | ||||
detected, configuring, err := github.ConfigureRepository(ctx, l.run, request, string(payload), l.logger) | ||||
if detected { | ||||
if configuring && err == nil { | ||||
l.writeResponse(response, http.StatusCreated, "configured") | ||||
return | ||||
} | ||||
if configuring && err != nil { | ||||
l.logger.Errorf("repository auto-configure has failed, err: %v", err) | ||||
l.writeResponse(response, http.StatusOK, "failed to configure") | ||||
sm43 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
return | ||||
} | ||||
l.writeResponse(response, http.StatusOK, "skipped event") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this would look weird in log can you write which event has been skipped? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't log this here, this is api response
for skipped repo events |
||||
return | ||||
} | ||||
} | ||||
|
||||
isIncoming, targettedRepo, err := l.detectIncoming(ctx, request, payload) | ||||
if err != nil { | ||||
l.logger.Errorf("error processing incoming webhook: %v", err) | ||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,112 @@ | ||||
package github | ||||
|
||||
import ( | ||||
"context" | ||||
"encoding/json" | ||||
"fmt" | ||||
"net/http" | ||||
|
||||
"github.com/google/go-github/v47/github" | ||||
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" | ||||
"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting" | ||||
"github.com/openshift-pipelines/pipelines-as-code/pkg/params" | ||||
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" | ||||
"github.com/openshift-pipelines/pipelines-as-code/pkg/templates" | ||||
"go.uber.org/zap" | ||||
corev1 "k8s.io/api/core/v1" | ||||
"k8s.io/apimachinery/pkg/api/errors" | ||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||
) | ||||
|
||||
const defaultNsTemplate = "%v-pipelines" | ||||
|
||||
func ConfigureRepository(ctx context.Context, run *params.Run, req *http.Request, payload string, logger *zap.SugaredLogger) (bool, bool, error) { | ||||
// gitea set x-github-event too, so skip it for the gitea driver | ||||
chmouel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
if h := req.Header.Get("X-Gitea-Event-Type"); h != "" { | ||||
return false, false, nil | ||||
} | ||||
event := req.Header.Get("X-Github-Event") | ||||
if event != "repository" { | ||||
return false, false, nil | ||||
} | ||||
|
||||
eventInt, err := github.ParseWebHook(event, []byte(payload)) | ||||
if err != nil { | ||||
return true, false, err | ||||
} | ||||
_ = json.Unmarshal([]byte(payload), &eventInt) | ||||
repoEvent, _ := eventInt.(*github.RepositoryEvent) | ||||
|
||||
if repoEvent.GetAction() != "created" { | ||||
chmouel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
logger.Infof("github: repository event \"%v\" is not supported", repoEvent.GetAction()) | ||||
return true, false, nil | ||||
} | ||||
|
||||
logger.Infof("github: configuring repository cr for repo: %v", repoEvent.Repo.GetHTMLURL()) | ||||
if err := createRepository(ctx, run.Info.Pac.AutoConfigureRepoNamespaceTemplate, run.Clients, repoEvent, logger); err != nil { | ||||
logger.Errorf("failed repository creation: %v", err) | ||||
return true, true, err | ||||
} | ||||
|
||||
return true, true, nil | ||||
} | ||||
|
||||
func createRepository(ctx context.Context, nsTemplate string, clients clients.Clients, gitEvent *github.RepositoryEvent, logger *zap.SugaredLogger) error { | ||||
repoNsName, err := generateNamespaceName(nsTemplate, gitEvent) | ||||
if err != nil { | ||||
return fmt.Errorf("failed to generate namespace for repo: %w", err) | ||||
} | ||||
|
||||
logger.Info("github: generated namespace name: ", repoNsName) | ||||
|
||||
// create namespace | ||||
repoNs := &corev1.Namespace{ | ||||
ObjectMeta: metav1.ObjectMeta{ | ||||
Name: repoNsName, | ||||
}, | ||||
} | ||||
repoNs, err = clients.Kube.CoreV1().Namespaces().Create(ctx, repoNs, metav1.CreateOptions{}) | ||||
if err != nil && !errors.IsAlreadyExists(err) { | ||||
return fmt.Errorf("failed to create namespace %v: %w", repoNs.Name, err) | ||||
} | ||||
|
||||
if errors.IsAlreadyExists(err) { | ||||
logger.Infof("github: namespace %v already exists, creating repository", repoNsName) | ||||
} else { | ||||
logger.Info("github: created repository namespace: ", repoNs.Name) | ||||
} | ||||
|
||||
// create repository | ||||
repo := &v1alpha1.Repository{ | ||||
ObjectMeta: metav1.ObjectMeta{ | ||||
Name: repoNsName, | ||||
Namespace: repoNsName, | ||||
}, | ||||
Spec: v1alpha1.RepositorySpec{ | ||||
URL: gitEvent.Repo.GetHTMLURL(), | ||||
}, | ||||
} | ||||
repo, err = clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(repoNsName).Create(ctx, repo, metav1.CreateOptions{}) | ||||
if err != nil { | ||||
return fmt.Errorf("failed to create repository for repo: %v: %w", gitEvent.Repo.GetHTMLURL(), err) | ||||
} | ||||
logger.Infof("github: repository created: %s/%s ", repo.Namespace, repo.Name) | ||||
return nil | ||||
} | ||||
|
||||
func generateNamespaceName(nsTemplate string, gitEvent *github.RepositoryEvent) (string, error) { | ||||
repoOwner, repoName, err := formatting.GetRepoOwnerSplitted(gitEvent.Repo.GetHTMLURL()) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GetRepoOwnerSplitter try to handle weird edge case for gitlab or other vcs naming, perhaps you can just use splitGithubURL which should be more robust for github There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GetRepoOwnerSplitter is doing a lot more things .. and it error out on normal git url like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. errors out here
not sure why that condition is <= 🤔 any reason to have =
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's at least org/repo/path, == would be no subpath too |
||||
if err != nil { | ||||
return "", fmt.Errorf("failed to parse git repo url: %w", err) | ||||
} | ||||
|
||||
if nsTemplate == "" { | ||||
return fmt.Sprintf(defaultNsTemplate, repoName), nil | ||||
} | ||||
|
||||
maptemplate := map[string]string{ | ||||
"repo_owner": repoOwner, | ||||
"repo_name": repoName, | ||||
} | ||||
return templates.ReplacePlaceHoldersVariables(nsTemplate, maptemplate), nil | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit annoying thing to give! I wish we could separate those capaibilities depending of feature being enabled or not...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it is possible to do through operator, if enabled then update the controller role.
not possible directly with PAC I think 🤔
but gets complicated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it complicated with operator or when doing it with PAC,
I wonder if we should just disable it by default, document it and let the operator handles it?
I don't think everyone would be using this feature....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with operator it will be easy actually depending on what user configure we an update the role
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#933