Skip to content

Commit

Permalink
Fix typos found in PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
Oded-B committed Jul 24, 2024
1 parent e0d7cd2 commit adc08ad
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 18 deletions.
4 changes: 2 additions & 2 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Configuration keys:
|`argocd.autoMergeNoDiffPRs`| if true, Telefonistka will **merge** promotion PRs that are not expected to change the target clusters. Requires `commentArgocdDiffonPR` and possibly `autoApprovePromotionPrs`(depending on repo branch protection rules)|
|`argocd.useSHALabelForAppDiscovery`| The default method for discovering relevant ArgoCD applications (for a PR) relies on fetching all applications in the repo and checking the `argocd.argoproj.io/manifest-generate-paths` **annotation**, this might cause a performance issue on a repo with a large number of ArgoCD applications. The alternative is to add SHA1 of the application path as a **label** and rely on ArgoCD server-side filtering, label name is `telefonistka.io/component-path-sha1`.|
|`argocd.allowSyncfromBranchPathRegex`| This controls which component(=ArgoCD apps) are allowed to be "applied" from a PR branch, by setting the ArgoCD application `Target Revision` to PR branch.|
|`argocd.createTempAppObjectFroNewApps`| For application created in PR Telefonistka needs to create a temporary ArgoCD Application Object to render the manifests, this key enables this behavior. The application spec is pulled from a Matching ApplicationSet object and the temporary object is deleted after the manifests are rendered. This feature currently support ApplicationSets with Git **Directory** generator|
|`argocd.createTempAppObjectFromNewApps`| For application created in PR Telefonistka needs to create a temporary ArgoCD Application Object to render the manifests, this key enables this behavior. The application spec is pulled from a Matching ApplicationSet object and the temporary object is deleted after the manifests are rendered. This feature currently support ApplicationSets with Git **Directory** generator|
<!-- markdownlint-enable MD033 -->

Example:
Expand Down Expand Up @@ -178,7 +178,7 @@ argocd:
autoMergeNoDiffPRs: true
allowSyncfromBranchPathRegex: '^workspace/.*$'
useSHALabelForAppDiscovery: true
createTempAppObjectFroNewApps: true
createTempAppObjectFromNewApps: true
toggleCommitStatus:
override-terrafrom-pipeline: "github-action-terraform"
```
Expand Down
27 changes: 13 additions & 14 deletions internal/pkg/argocd/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ type DiffElement struct {

// DiffResult struct to store diff result
type DiffResult struct {
ComponentPath string
ArgoCdAppName string
ArgoCdAppURL string
DiffElements []DiffElement
HasDiff bool
DiffError error
AppWasTemporarlyCreated bool
ComponentPath string
ArgoCdAppName string
ArgoCdAppURL string
DiffElements []DiffElement
HasDiff bool
DiffError error
AppWasTemporarilyCreated bool
}

// Mostly copied from https://github.com/argoproj/argo-cd/blob/4f6a8dce80f0accef7ed3b5510e178a6b398b331/cmd/argocd/commands/app.go#L1255C6-L1338
Expand Down Expand Up @@ -402,12 +402,11 @@ func createTempAppObjectFroNewApp(ctx context.Context, componentPath string, rep

return app, err
} else {
log.Errorf("Didn't find an application nor a relevant ApplicationSet: %v", err)
return nil, err
}
}

func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPath string, prBranch string, repo string, ac argoCdClients, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool, createTempAppObjectFroNewApps bool) (componentDiffResult DiffResult) {
func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPath string, prBranch string, repo string, ac argoCdClients, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool, createTempAppObjectFromNewApps bool) (componentDiffResult DiffResult) {
componentDiffResult.ComponentPath = componentPath

// Find ArgoCD application by the path SHA1 label selector and repo name
Expand All @@ -429,7 +428,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa
}
}
if app == nil {
if createTempAppObjectFroNewApps {
if createTempAppObjectFromNewApps {
app, err = createTempAppObjectFroNewApp(ctx, componentPath, repo, prBranch, ac)

if err != nil {
Expand All @@ -438,7 +437,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa
return componentDiffResult
} else {
log.Debugf("Created temporary app object: %s", app.Name)
componentDiffResult.AppWasTemporarlyCreated = true
componentDiffResult.AppWasTemporarilyCreated = true
}
} else {
componentDiffResult.DiffError = fmt.Errorf("No ArgoCD application found for component path %s(repo %s)", componentPath, repo)
Expand Down Expand Up @@ -505,7 +504,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa
log.Debugf("Generating diff for component %s", componentPath)
componentDiffResult.HasDiff, componentDiffResult.DiffElements, componentDiffResult.DiffError = generateArgocdAppDiff(ctx, commentDiff, app, detailedProject.Project, resources, argoSettings, diffOption)

if componentDiffResult.AppWasTemporarlyCreated {
if componentDiffResult.AppWasTemporarilyCreated {
// Delete the temporary app object
_, err = ac.app.Delete(ctx, &application.ApplicationDeleteRequest{Name: &app.Name, AppNamespace: &app.Namespace})
if err != nil {
Expand All @@ -520,7 +519,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa
}

// GenerateDiffOfChangedComponents generates diff of changed components
func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[string]bool, prBranch string, repo string, useSHALabelForArgoDicovery bool, createTempAppObjectFroNewApps bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[string]bool, prBranch string, repo string, useSHALabelForArgoDicovery bool, createTempAppObjectFromNewApps bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
hasComponentDiff = false
hasComponentDiffErrors = false
// env var should be centralized
Expand All @@ -537,7 +536,7 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[s
}

for componentPath, shouldIDiff := range componentsToDiff {
currentDiffResult := generateDiffOfAComponent(ctx, shouldIDiff, componentPath, prBranch, repo, ac, argoSettings, useSHALabelForArgoDicovery, createTempAppObjectFroNewApps)
currentDiffResult := generateDiffOfAComponent(ctx, shouldIDiff, componentPath, prBranch, repo, ac, argoSettings, useSHALabelForArgoDicovery, createTempAppObjectFromNewApps)
if currentDiffResult.DiffError != nil {
log.Errorf("Error generating diff for component %s: %v", componentPath, currentDiffResult.DiffError)
hasComponentDiffErrors = true
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/configuration/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type ArgocdConfig struct {
AutoMergeNoDiffPRs bool `yaml:"autoMergeNoDiffPRs"`
AllowSyncfromBranchPathRegex string `yaml:"allowSyncfromBranchPathRegex"`
UseSHALabelForAppDiscovery bool `yaml:"useSHALabelForAppDiscovery"`
CreateTempAppObjectFroNewApps bool `yaml:"createTempAppObjectFroNewApps"`
CreateTempAppObjectFroNewApps bool `yaml:"createTempAppObjectFromNewApps"`
}

func ParseConfigFromYaml(y string) (*Config, error) {
Expand Down
2 changes: 1 addition & 1 deletion templates/argoCD-diff-pr-comment.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Diff of ArgoCD applications:
{{- else }}
No diff 🤷
{{- end}}
{{if $appDiffResult.AppWasTemporarlyCreated }}
{{if $appDiffResult.AppWasTemporarilyCreated }}
⚠️ ⚠️ ⚠️
This PR appears to create this new application, Telefonistka has **temporarly** created an ArgoCD app object for it just to render its manifests.
It will not be present in ArgoCD UI for more than a few seconds and it can not be synced from the PR branch
Expand Down

0 comments on commit adc08ad

Please sign in to comment.