Skip to content

Commit

Permalink
fix: surface source not permitted errors (#12255)
Browse files Browse the repository at this point in the history 
* chore: surface source not permitted errors

For Git and Helm repositories, we filter out non-permitted urls before
submitting a `ManifestRequest` to the repo-server. While that works
fine, this also leads to very hard to debug issues in particular when
using Helm dependencies.

This (very) WIP PR adds `ProjectSourceRepos` as a parameter to
`ManifestRequest`, so we can verify that a source is in fact
permitted in order to distinguish between actual 40x errors (caused
by e.g misconfiguration) and "source not permitted" caused by not
adding the relevant sources to the AppProject config.

This still needs documentation, tests and some basic sanity checking
before proceeding further, as well as resolving whatever is causing
`make codegen` to not properly work.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: propagate project values to repo-server

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: fix failing unit tests

now onto the e2e tests...

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: fix failing e2e test(s)

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: add project params

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: add e2e test

Add Helm dependency check test.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: remove git source check

Discussed over Slack and deemed this to not be necessary at this time.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* make codegen

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: cr tweaks

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: code review tweaks

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: fix

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: wip

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: wip

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: wip

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: typo

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: typo

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: rebase fixes

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: oci:// is not prefixed

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

---------

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
  • Loading branch information
@blakepettersson
blakepettersson authored May 27, 2023
1 parent 4ce4885 commit c651bd8
Show file tree
Hide file tree
Showing 11 changed files with 511 additions and 200 deletions.
46 changes: 27 additions & 19 deletions cmd/argocd/commands/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,9 +845,9 @@ func targetObjects(resources []*argoappv1.ResourceDiff) ([]*unstructured.Unstruc
return objs, nil
}

func getLocalObjects(ctx context.Context, app *argoappv1.Application, local, localRepoRoot, appLabelKey, kubeVersion string, apiVersions []string, kustomizeOptions *argoappv1.KustomizeOptions,
func getLocalObjects(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, local, localRepoRoot, appLabelKey, kubeVersion string, apiVersions []string, kustomizeOptions *argoappv1.KustomizeOptions,
configManagementPlugins []*argoappv1.ConfigManagementPlugin, trackingMethod string) []*unstructured.Unstructured {
manifestStrings := getLocalObjectsString(ctx, app, local, localRepoRoot, appLabelKey, kubeVersion, apiVersions, kustomizeOptions, configManagementPlugins, trackingMethod)
manifestStrings := getLocalObjectsString(ctx, app, proj, local, localRepoRoot, appLabelKey, kubeVersion, apiVersions, kustomizeOptions, configManagementPlugins, trackingMethod)
objs := make([]*unstructured.Unstructured, len(manifestStrings))
for i := range manifestStrings {
obj := unstructured.Unstructured{}
Expand All @@ -858,20 +858,22 @@ func getLocalObjects(ctx context.Context, app *argoappv1.Application, local, loc
return objs
}

func getLocalObjectsString(ctx context.Context, app *argoappv1.Application, local, localRepoRoot, appLabelKey, kubeVersion string, apiVersions []string, kustomizeOptions *argoappv1.KustomizeOptions,
func getLocalObjectsString(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, local, localRepoRoot, appLabelKey, kubeVersion string, apiVersions []string, kustomizeOptions *argoappv1.KustomizeOptions,
configManagementPlugins []*argoappv1.ConfigManagementPlugin, trackingMethod string) []string {
source := app.Spec.GetSource()
res, err := repository.GenerateManifests(ctx, local, localRepoRoot, source.TargetRevision, &repoapiclient.ManifestRequest{
Repo: &argoappv1.Repository{Repo: source.RepoURL},
AppLabelKey: appLabelKey,
AppName: app.Name,
Namespace: app.Spec.Destination.Namespace,
ApplicationSource: &source,
KustomizeOptions: kustomizeOptions,
KubeVersion: kubeVersion,
ApiVersions: apiVersions,
Plugins: configManagementPlugins,
TrackingMethod: trackingMethod,
Repo: &argoappv1.Repository{Repo: source.RepoURL},
AppLabelKey: appLabelKey,
AppName: app.Name,
Namespace: app.Spec.Destination.Namespace,
ApplicationSource: &source,
KustomizeOptions: kustomizeOptions,
KubeVersion: kubeVersion,
ApiVersions: apiVersions,
Plugins: configManagementPlugins,
TrackingMethod: trackingMethod,
ProjectName: proj.Name,
ProjectSourceRepos: proj.Spec.SourceRepos,
}, true, &git.NoopCredsStore{}, resource.MustParse("0"), nil)
errors.CheckError(err)

Expand Down Expand Up @@ -989,7 +991,8 @@ func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
diffOption.cluster = cluster
}
}
foundDiffs := findandPrintDiff(ctx, app, resources, argoSettings, appName, diffOption)
proj := getProject(c, clientOpts, ctx, app.Spec.Project)
foundDiffs := findandPrintDiff(ctx, app, proj.Project, resources, argoSettings, appName, diffOption)
if foundDiffs && exitCode {
os.Exit(1)
}
Expand Down Expand Up @@ -1017,13 +1020,13 @@ type DifferenceOption struct {
}

// findandPrintDiff ... Prints difference between application current state and state stored in git or locally, returns boolean as true if difference is found else returns false
func findandPrintDiff(ctx context.Context, app *argoappv1.Application, resources *application.ManagedResourcesResponse, argoSettings *settingspkg.Settings, appName string, diffOptions *DifferenceOption) bool {
func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, resources *application.ManagedResourcesResponse, argoSettings *settingspkg.Settings, appName string, diffOptions *DifferenceOption) bool {
var foundDiffs bool
liveObjs, err := cmdutil.LiveObjects(resources.Items)
errors.CheckError(err)
items := make([]objKeyLiveTarget, 0)
if diffOptions.local != "" {
localObjs := groupObjsByKey(getLocalObjects(ctx, app, diffOptions.local, diffOptions.localRepoRoot, argoSettings.AppLabelKey, diffOptions.cluster.Info.ServerVersion, diffOptions.cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod), liveObjs, app.Spec.Destination.Namespace)
localObjs := groupObjsByKey(getLocalObjects(ctx, app, proj, diffOptions.local, diffOptions.localRepoRoot, argoSettings.AppLabelKey, diffOptions.cluster.Info.ServerVersion, diffOptions.cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod), liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, localObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace))
} else if diffOptions.revision != "" {
var unstructureds []*unstructured.Unstructured
Expand Down Expand Up @@ -1686,7 +1689,9 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
cluster, err := clusterIf.Get(ctx, &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
errors.CheckError(err)
argoio.Close(conn)
localObjsStrings = getLocalObjectsString(ctx, app, local, localRepoRoot, argoSettings.AppLabelKey, cluster.Info.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod)

proj := getProject(c, clientOpts, ctx, app.Spec.Project)
localObjsStrings = getLocalObjectsString(ctx, app, proj.Project, local, localRepoRoot, argoSettings.AppLabelKey, cluster.Info.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod)
errors.CheckError(err)
diffOption.local = local
diffOption.localRepoRoot = localRepoRoot
Expand Down Expand Up @@ -1755,7 +1760,9 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
errors.CheckError(err)
foundDiffs := false
fmt.Printf("====== Previewing differences between live and desired state of application %s ======\n", appQualifiedName)
foundDiffs = findandPrintDiff(ctx, app, resources, argoSettings, appQualifiedName, diffOption)

proj := getProject(c, clientOpts, ctx, app.Spec.Project)
foundDiffs = findandPrintDiff(ctx, app, proj.Project, resources, argoSettings, appQualifiedName, diffOption)
if foundDiffs {
if !diffChangesConfirm {
yesno := cli.AskToProceed(fmt.Sprintf("Please review changes to application %s shown above. Do you want to continue the sync process? (y/n): ", appQualifiedName))
Expand Down Expand Up @@ -2367,7 +2374,8 @@ func NewApplicationManifestsCommand(clientOpts *argocdclient.ClientOptions) *cob
cluster, err := clusterIf.Get(context.Background(), &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
errors.CheckError(err)

unstructureds = getLocalObjects(context.Background(), app, local, localRepoRoot, argoSettings.AppLabelKey, cluster.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod)
proj := getProject(c, clientOpts, ctx, app.Spec.Project)
unstructureds = getLocalObjects(context.Background(), app, proj.Project, local, localRepoRoot, argoSettings.AppLabelKey, cluster.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod)
} else if revision != "" {
q := application.ApplicationManifestQuery{
Name: &appName,
Expand Down
14 changes: 10 additions & 4 deletions cmd/argocd/commands/project.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package commands

import (
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -819,10 +820,7 @@ func NewProjectGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command
os.Exit(1)
}
projName := args[0]
conn, projIf := headless.NewClientOrDie(clientOpts, c).NewProjectClientOrDie()
defer argoio.Close(conn)
detailedProject, err := projIf.GetDetailedProject(ctx, &projectpkg.ProjectQuery{Name: projName})
errors.CheckError(err)
detailedProject := getProject(c, clientOpts, ctx, projName)

switch output {
case "yaml", "json":
Expand All @@ -839,6 +837,14 @@ func NewProjectGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command
return command
}

func getProject(c *cobra.Command, clientOpts *argocdclient.ClientOptions, ctx context.Context, projName string) *projectpkg.DetailedProjectsResponse {
conn, projIf := headless.NewClientOrDie(clientOpts, c).NewProjectClientOrDie()
defer argoio.Close(conn)
detailedProject, err := projIf.GetDetailedProject(ctx, &projectpkg.ProjectQuery{Name: projName})
errors.CheckError(err)
return detailedProject
}

func NewProjectEditCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
var command = &cobra.Command{
Use: "edit PROJECT",
Expand Down
2 changes: 2 additions & 0 deletions controller/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
HelmOptions: helmOptions,
HasMultipleSources: app.Spec.HasMultipleSources(),
RefSources: refSources,
ProjectName: proj.Name,
ProjectSourceRepos: proj.Spec.SourceRepos,
})
if err != nil {
return nil, nil, err
Expand Down
Loading

0 comments on commit c651bd8

Please sign in to comment.