Skip to content

Commit

Permalink
fix(reposerver): loosen source not permitted helm errors (argoproj#14210
Browse files Browse the repository at this point in the history 
)

* fix: loosen source not permitted helm errors

With argoproj#12255, we check if a source is first permitted before running
`helm template`. This works a bit too well, since this may break
previously working manifests. If an `AppProject` has a set of
`sourceRepos` which are more restrictive than `*`, and it also has Helm
public dependencies (repos with credentials would not work with 2.7x
due to the fact they get filtered out before ending up on the repo
server). Whereas before this would work, this currently fails on
`HEAD` but not in `2.7x`.

What we instead do here is that we only run this check if the chart
failed to download - if it does then we run a check to see if the repo
is in the allowed repos list. If the repo is not in the allowed repos
list, we return the same error as in argoproj#12555, otherwise we bubble up the
error.

Should fix argoproj#13833.

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

* fix: check for 401 unauthorized in error

The regex check works fine for OCI artifacts, but the flow is slightly
different for standard Helm charts (specifically when running
`helm repo add`). To get around that, we also check the error for
`401 Unauthorized`.

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

* fix: loosen string check

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

* Revert "chore: revert argoproj#12255 (argoproj#14858)"

This reverts commit c8ae5bc.

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

* wip

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

* wip

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

* chore: reword test to reduce confusion

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

---------

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: jmilic1 <70441727+jmilic1@users.noreply.github.com>
  • Loading branch information
@blakepettersson @jmilic1
blakepettersson authored and jmilic1 committed Nov 13, 2023
1 parent 8565c6a commit bde494f
Show file tree
Hide file tree
Showing 12 changed files with 524 additions and 215 deletions.
42 changes: 24 additions & 18 deletions cmd/argocd/commands/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,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,
trackingMethod string) []*unstructured.Unstructured {
manifestStrings := getLocalObjectsString(ctx, app, local, localRepoRoot, appLabelKey, kubeVersion, apiVersions, kustomizeOptions, trackingMethod)
manifestStrings := getLocalObjectsString(ctx, app, proj, local, localRepoRoot, appLabelKey, kubeVersion, apiVersions, kustomizeOptions, trackingMethod)
objs := make([]*unstructured.Unstructured, len(manifestStrings))
for i := range manifestStrings {
obj := unstructured.Unstructured{}
Expand All @@ -859,19 +859,21 @@ 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,
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,
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,
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, diffOption)
proj := getProject(c, clientOpts, ctx, app.Spec.Project)
foundDiffs := findandPrintDiff(ctx, app, proj.Project, resources, argoSettings, 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 *settings.Settings, diffOptions *DifferenceOption) bool {
func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, resources *application.ManagedResourcesResponse, argoSettings *settings.Settings, 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.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.TrackingMethod), liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, localObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
} else if diffOptions.revision != "" {
var unstructureds []*unstructured.Unstructured
Expand Down Expand Up @@ -1694,7 +1697,8 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
errors.CheckError(err)
argoio.Close(conn)

localObjsStrings = getLocalObjectsString(ctx, app, local, localRepoRoot, argoSettings.AppLabelKey, cluster.Info.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, 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.TrackingMethod)
errors.CheckError(err)
diffOption.local = local
diffOption.localRepoRoot = localRepoRoot
Expand Down Expand Up @@ -1764,7 +1768,8 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
foundDiffs := false
fmt.Printf("====== Previewing differences between live and desired state of application %s ======\n", appQualifiedName)

foundDiffs = findandPrintDiff(ctx, app, resources, argoSettings, diffOption)
proj := getProject(c, clientOpts, ctx, app.Spec.Project)
foundDiffs = findandPrintDiff(ctx, app, proj.Project, resources, argoSettings, 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 @@ -2376,7 +2381,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.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.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 @@ -818,10 +819,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 @@ -838,6 +836,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 @@ -200,6 +200,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, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
Expand Down
Loading

0 comments on commit bde494f

Please sign in to comment.