diff --git a/api/v1alpha1/config.go b/api/v1alpha1/config.go index 9a9443d..2918854 100644 --- a/api/v1alpha1/config.go +++ b/api/v1alpha1/config.go @@ -2,8 +2,9 @@ package v1alpha1 import "strings" -// Config stores the hydros configuration. -type Config struct { +// HydrosConfig is hydros GitHub App configuration. This is the configuration that should be checked into +// a repository to configure hydros on that repository +type HydrosConfig struct { APIVersion string `yaml:"apiVersion"` Kind string `yaml:"kind"` Metadata Metadata `yaml:"metadata"` @@ -35,7 +36,7 @@ type InPlaceConfig struct { // IsValid returns true if the config is valid. // For invalid config the string will be a message of validation errors -func IsValid(c *Config) (string, bool) { +func IsValid(c *HydrosConfig) (string, bool) { errors := make([]string, 0, 10) baseBranches := make(map[string]bool) prBranches := make(map[string]bool) @@ -53,7 +54,7 @@ func IsValid(c *Config) (string, bool) { } if len(errors) > 0 { - return "Config is invalid. " + strings.Join(errors, ". "), false + return "HydrosConfig is invalid. " + strings.Join(errors, ". "), false } return "", true } diff --git a/pkg/app/config.go b/pkg/app/config.go index 37eb1a3..461886d 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -39,7 +39,11 @@ type App struct { PrivateKey string `yaml:"private_key" json:"privateKey"` } -// BuildConfig is a helper function to build the configuration +// BuildConfig is a helper function to build the GitHubApp configuration. +// Palantir libraries use *githubapp.Config for basic configuration. We don't necessarily want to directly use +// that configuration because not all of those options make sense. For example, we allow URIs to be used +// for the secrets. So this is a helper function to convert our values into githubapp.Config which +// can be passed to those libraries. func BuildConfig(appId int64, webhookSecret string, privateKeySecret string) (*githubapp.Config, error) { hmacSecret, err := readSecret(webhookSecret) if err != nil { diff --git a/pkg/app/fetcher.go b/pkg/app/fetcher.go index b874d75..d5597e3 100644 --- a/pkg/app/fetcher.go +++ b/pkg/app/fetcher.go @@ -12,7 +12,7 @@ import ( // FetchedConfig represents the hydros configuration fetched from GitHub and used to configure hydros type FetchedConfig struct { - Config *v1alpha1.Config + Config *v1alpha1.HydrosConfig LoadError error ParseError error @@ -39,7 +39,7 @@ func (cf *ConfigFetcher) ConfigForRepositoryBranch(ctx context.Context, client * return fc } - var pc v1alpha1.Config + var pc v1alpha1.HydrosConfig if err := yaml.UnmarshalStrict(c.Content, &pc); err != nil { fc.ParseError = err } else { diff --git a/pkg/app/handler.go b/pkg/app/handler.go index 929ecd5..1d25761 100644 --- a/pkg/app/handler.go +++ b/pkg/app/handler.go @@ -31,6 +31,9 @@ const ( SharedRepository = ".github" ) +// HydrosHandler is a handler for certain GitHub events. It currently handles PushEvents by sending them to +// Renderer which knows how to do in place modification using KRMs. +// TODO(jeremy): Also handle syncer. type HydrosHandler struct { githubapp.ClientCreator Manager *gitops.Manager @@ -38,12 +41,12 @@ type HydrosHandler struct { transports *hGithub.TransportManager workDir string - fetcher *ConfigFetcher } // NewHandler starts a new HydrosHandler for GitHub. -// config: The GitHub configuration +// cc: ClientCreator for creating GitHub clients. +// transports: Manage GitHub transports // workDir: The directory to use for storing temporary files and checking out repositories // numWorkers: The number of workers to use for processing events func NewHandler(cc githubapp.ClientCreator, transports *hGithub.TransportManager, workDir string, numWorkers int) (*HydrosHandler, error) { @@ -90,11 +93,14 @@ func (h *HydrosHandler) Handles() []string { } func (h *HydrosHandler) Handle(ctx context.Context, eventType, deliveryID string, payload []byte) error { - // TODO(jeremy): How do we distinguish between pushing a PR and merging a PR. log := zapr.NewLogger(zap.L()).WithValues("eventType", eventType, "deliverID", deliveryID) log.V(util.Debug).Info("Got github webhook") r := bytes.NewBuffer(payload) d := json.NewDecoder(r) + + // Handle push events. + // GitHub push events are sent whenever a branch is pushed. I believe a single push could contain multiple commits. + // When a PR is merged, that will trigger a push with the commits pushed to the target branch. event := &github.PushEvent{} if err := d.Decode(event); err != nil { log.Error(err, "Failed to decode a PushEvent") @@ -110,7 +116,6 @@ func (h *HydrosHandler) Handle(ctx context.Context, eventType, deliveryID string return err } - // TODO(jeremy): We can use the checks client to create checks. installationID := githubapp.GetInstallationIDFromEvent(event) client, err := h.NewInstallationClient(installationID) if err != nil { @@ -192,21 +197,11 @@ func (h *HydrosHandler) Handle(ctx context.Context, eventType, deliveryID string rName := gitops.RendererName(repoName.RepoOwner(), repoName.RepoName()) if !h.Manager.HasReconciler(rName) { - - if ghrepo.FullName(repoName) != "jlewi/hydros-hydrated" { - return errors.Errorf("Currently only the repository jlewi/hydros-hydrated. Code needs to be updated to support other repositories") - } log.Info("Creating reconciler", "name", rName) - - // TODO(jeremy): This information should come from the configuration checked into the repository. - // e.g.from the file .github/hydros.yaml - - org := "jlewi" - repo := "hydros-hydrated" // Make sure workdir is unique for each reconciler. workDir := filepath.Join(h.workDir, rName) - r, err := gitops.NewRenderer(org, repo, workDir, h.transports, client) + r, err := gitops.NewRenderer(repoName.RepoOwner(), repoName.RepoName(), workDir, h.transports, client) if err != nil { return err } @@ -224,7 +219,7 @@ func (h *HydrosHandler) Handle(ctx context.Context, eventType, deliveryID string // https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads#push // "After" is the commit after the push. Commit: event.GetAfter(), - // Config could potentially be different for different commits + // HydrosConfig could potentially be different for different commits // So we pass it along with the event BranchConfig: inPlaceConfig, }) diff --git a/pkg/app/handler_test.go b/pkg/app/handler_test.go index 91af8d3..7c29c3c 100644 --- a/pkg/app/handler_test.go +++ b/pkg/app/handler_test.go @@ -91,7 +91,6 @@ func Test_HookManual(t *testing.T) { t.Fatalf("Failed to handle the push") } - //time.Sleep(10 * time.Minute) // Wait for it to finish processing. handler.Manager.Shutdown() } @@ -176,9 +175,6 @@ func latestCommit(transports *hGithub.TransportManager, repo ghrepo.Interface, w return "", err } - // The short tag will be used to tag the artifacts - //b.commitTag = ref.Hash().String()[0:7] - log.Info("Current commit", "commit", ref.Hash().String()) return ref.Hash().String(), nil } diff --git a/pkg/gitops/renderer.go b/pkg/gitops/renderer.go index 7d1f784..12f3396 100644 --- a/pkg/gitops/renderer.go +++ b/pkg/gitops/renderer.go @@ -26,7 +26,7 @@ const ( RendererCheckName = "hydros-ai" ) -// Renderer handles in place modification of YAML files. +// Renderer is a reconciler that handles in place modification of YAML files. // It is intended to run a bunch of KRM functions in place and then check the modifications back into the repository. // // There is currently one renderer per repository. A single renderer can handle multiple branches but not @@ -38,19 +38,9 @@ const ( // when hydrating into a different repository (e.g. via Syncer) but not when changes are to be checked into the // source repository. type Renderer struct { - org string - repo string - // ForkRepo is the repo into which the changes will be pushed and the PR created from - //ForkRepo *v1alpha1.GitHubRepo `yaml:"forkRepo,omitempty"` - // - //// DestRepo is the repo into which a PR will be created to merge hydrated - //// manifests from the ForkRepo - //DestRepo *v1alpha1.GitHubRepo `yaml:"destRepo,omitempty"` - - workDir string - - // repoHelper helps with creating PRs - // repoHelper *github.RepoHelper + org string + repo string + workDir string transports *github.TransportManager client *ghAPI.Client @@ -58,8 +48,6 @@ type Renderer struct { func NewRenderer(org string, name string, workDir string, transports *github.TransportManager, client *ghAPI.Client) (*Renderer, error) { r := &Renderer{ - //ForkRepo: forkRepo, - //DestRepo: destRepo, org: org, repo: name, workDir: workDir, @@ -99,6 +87,9 @@ func (r *Renderer) Run(anyEvent any) error { return fmt.Errorf("Event is not a RenderEvent") } + // TODO(https://github.com/jlewi/hydros/issues/32): The semantics should probably be that we skip running if + // we aren't the latest commit. So if the commit is specified we should verify its the latest commit and if not + // skip it. if event.Commit == "" { // When no commit is specified we should run on the latest commit. This means // 1. We need to fetch the latest commit @@ -209,17 +200,17 @@ func (r *Renderer) Run(anyEvent any) error { } } - if event.Commit != "" { - // TODO(jeremy): We need to update PrepareBranch to properly create the branch from the commit. - err := errors.Errorf("Commit isn't properly supported yet. The branch is prepared off HEAD and not the commit") - log.Error(err, "Commit isn't properly supported yet.", "commit", event.Commit) - } - // Checkout the repository. if err := repoHelper.PrepareBranch(true); err != nil { return err } + if event.Commit != "" { + // TODO(https://github.com/jlewi/hydros/issues/32): We should check whether commit is the latest + // commit on the branch and if it isn't we should the run. + log.Info("Warning. Commit is set in RenderEvent but code will use code from latest commit regardless of whether it matches the commit. See https://github.com/jlewi/hydros/issues/32.", "commit", event.Commit) + } + syncNeeded, err := r.syncNeeded() if err != nil { return err diff --git a/pkg/gitops/renderer_test.go b/pkg/gitops/renderer_test.go index ebad8d8..beacb55 100644 --- a/pkg/gitops/renderer_test.go +++ b/pkg/gitops/renderer_test.go @@ -49,28 +49,19 @@ func Test_RendererManualE2E(t *testing.T) { } r := Renderer{ - //SourceRepo: &v1alpha1.GitHubRepo{ - // Org: "jlewi", - // Repo: "hydros", - // Branch: "jlewi/ai", - //}, - ForkRepo: &v1alpha1.GitHubRepo{ - Org: "jlewi", - Repo: "hydros", - Branch: "jlewi/ai-test", - }, - DestRepo: &v1alpha1.GitHubRepo{ - Org: "jlewi", - Repo: "hydros", - Branch: "jlewi/ai", - }, workDir: "/tmp/test_renderer", - sourcePath: "tests/manifests", - repoHelper: nil, transports: manager, } - return r.Run() + event := RenderEvent{ + Commit: "", + BranchConfig: &v1alpha1.InPlaceConfig{ + BaseBranch: "main", + PRBranch: "hydros/manual-test", + AutoMerge: false, + }, + } + return r.Run(event) }() if err != nil { t.Fatalf("Error running renderer; %v", err)