Skip to content
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

feat: server-side manifest generation for local diff (#8145) #10019

Merged
merged 26 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7716d2c
feat: server-side manifest generation for diff (#8145)
notfromstatefarm Jul 17, 2022
b864cdc
fix docs, mocks, ineffectual err
notfromstatefarm Jul 18, 2022
1ca3f48
fix CMPs, ineffectual err
notfromstatefarm Jul 18, 2022
93b056a
refactor
notfromstatefarm Jul 18, 2022
c00cefe
add unit tests
notfromstatefarm Jul 18, 2022
ad4bcac
handle err
notfromstatefarm Jul 18, 2022
7aede3b
add size limits and inclusion filters
notfromstatefarm Jul 19, 2022
7f6c57c
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Jul 19, 2022
6bd57d2
fix docs
notfromstatefarm Jul 19, 2022
a2c0e29
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Jul 19, 2022
f42d09b
fix errors, increase defaults
notfromstatefarm Jul 20, 2022
57bdd12
use quantity, wrap errors, add security fields to logs, deprecation w…
notfromstatefarm Jul 20, 2022
9f5837f
have e2e test use server side generation
notfromstatefarm Jul 20, 2022
871a956
nits
notfromstatefarm Jul 20, 2022
00c5db4
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Jul 25, 2022
21b6c08
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Aug 9, 2022
b8b96f6
remove unused import
notfromstatefarm Aug 16, 2022
8d6b4c3
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Aug 16, 2022
a275f5d
fix merge conflict
notfromstatefarm Aug 16, 2022
ca4a9de
fix conflicts
notfromstatefarm Aug 16, 2022
e4232a6
fix e2e test
notfromstatefarm Aug 16, 2022
e0dc548
add deprecation/breaking change info
notfromstatefarm Aug 17, 2022
16fb445
remove security logging stuff, will be in a separate PR
notfromstatefarm Aug 17, 2022
822ae10
more specific docs
notfromstatefarm Aug 17, 2022
734691a
Merge branch 'master' into feat/server-side-diff
notfromstatefarm Aug 17, 2022
a1d488d
add security logging
notfromstatefarm Aug 17, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 65 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,40 @@
}
}
},
"/api/v1/applications/manifestsWithFiles": {
"post": {
"tags": [
"ApplicationService"
],
"summary": "GetManifestsWithFiles returns application manifests using provided files to generate them",
"operationId": "ApplicationService_GetManifestsWithFiles",
"parameters": [
{
"description": " (streaming inputs)",
"name": "body",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/applicationApplicationManifestQueryWithFilesWrapper"
}
}
],
"responses": {
"200": {
"description": "A successful response.",
"schema": {
"$ref": "#/definitions/repositoryManifestResponse"
}
},
"default": {
"description": "An unexpected error response.",
"schema": {
"$ref": "#/definitions/runtimeError"
}
}
}
}
},
"/api/v1/applications/{application.metadata.name}": {
"put": {
"tags": [
Expand Down Expand Up @@ -3386,6 +3420,28 @@
"accountUpdatePasswordResponse": {
"type": "object"
},
"applicationApplicationManifestQueryWithFiles": {
"type": "object",
"properties": {
"checksum": {
"type": "string"
},
"name": {
"type": "string"
}
}
},
"applicationApplicationManifestQueryWithFilesWrapper": {
"type": "object",
"properties": {
"chunk": {
"$ref": "#/definitions/applicationFileChunk"
},
"query": {
"$ref": "#/definitions/applicationApplicationManifestQueryWithFiles"
}
}
},
"applicationApplicationPatchRequest": {
"type": "object",
"title": "ApplicationPatchRequest is a request to patch an application",
Expand Down Expand Up @@ -3512,6 +3568,15 @@
}
}
},
"applicationFileChunk": {
"type": "object",
"properties": {
"chunk": {
"type": "string",
"format": "byte"
}
}
},
"applicationLogEntry": {
"type": "object",
"properties": {
Expand Down
7 changes: 6 additions & 1 deletion cmd/argocd-repo-server/commands/argocd_repo_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ func NewCommand() *cobra.Command {
maxCombinedDirectoryManifestsSize string
cmpTarExcludedGlobs []string
allowOutOfBoundsSymlinks bool
streamedManifestMaxTarSize int64
streamedManifestMaxExtractedSize int64
)
var command = cobra.Command{
Use: cliName,
Expand Down Expand Up @@ -126,6 +128,8 @@ func NewCommand() *cobra.Command {
MaxCombinedDirectoryManifestsSize: maxCombinedDirectoryManifestsQuantity,
CMPTarExcludedGlobs: cmpTarExcludedGlobs,
AllowOutOfBoundsSymlinks: allowOutOfBoundsSymlinks,
StreamedManifestMaxExtractedSize: streamedManifestMaxExtractedSize,
StreamedManifestMaxTarSize: streamedManifestMaxTarSize,
}, askPassServer)
errors.CheckError(err)

Expand Down Expand Up @@ -204,7 +208,8 @@ func NewCommand() *cobra.Command {
command.Flags().StringVar(&maxCombinedDirectoryManifestsSize, "max-combined-directory-manifests-size", env.StringFromEnv("ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE", "10M"), "Max combined size of manifest files in a directory-type Application")
command.Flags().StringArrayVar(&cmpTarExcludedGlobs, "plugin-tar-exclude", env.StringsFromEnv("ARGOCD_REPO_SERVER_PLUGIN_TAR_EXCLUSIONS", []string{}, ";"), "Globs to filter when sending tarballs to plugins.")
command.Flags().BoolVar(&allowOutOfBoundsSymlinks, "allow-oob-symlinks", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS", false), "Allow out-of-bounds symlinks in repositories (not recommended)")

command.Flags().Int64Var(&streamedManifestMaxTarSize, "streamed-manifest-max-tar-size", env.ParseInt64FromEnv("ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE", 1024*1024*100, 0, math.MaxInt64), "Maximum size of streamed manifest archives")
command.Flags().Int64Var(&streamedManifestMaxExtractedSize, "streamed-manifest-max-extracted-size", env.ParseInt64FromEnv("ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE", 1024*1024*1000, 0, math.MaxInt64), "Maximum size of streamed manifest archives when extracted")
notfromstatefarm marked this conversation as resolved.
Show resolved Hide resolved
tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command)
cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, func(client *redis.Client) {
redisClient = client
Expand Down
58 changes: 43 additions & 15 deletions cmd/argocd/commands/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/argoproj/gitops-engine/pkg/sync/ignore"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
"github.com/ghodss/yaml"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
"github.com/mattn/go-isatty"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -49,6 +50,7 @@ import (
"github.com/argoproj/argo-cd/v2/util/errors"
"github.com/argoproj/argo-cd/v2/util/git"
argoio "github.com/argoproj/argo-cd/v2/util/io"
"github.com/argoproj/argo-cd/v2/util/manifeststream"
"github.com/argoproj/argo-cd/v2/util/templates"
"github.com/argoproj/argo-cd/v2/util/text/label"
)
Expand Down Expand Up @@ -819,12 +821,14 @@ type objKeyLiveTarget struct {
// NewApplicationDiffCommand returns a new instance of an `argocd app diff` command
func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
var (
refresh bool
hardRefresh bool
exitCode bool
local string
revision string
localRepoRoot string
refresh bool
hardRefresh bool
exitCode bool
local string
revision string
localRepoRoot string
serverSideGenerate bool
localIncludes []string
)
shortDesc := "Perform a diff against the target and live state."
var command = &cobra.Command{
Expand Down Expand Up @@ -860,15 +864,27 @@ func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
errors.CheckError(err)
diffOption.res = res
diffOption.revision = revision
}
if local != "" {
conn, clusterIf := clientset.NewClusterClientOrDie()
defer argoio.Close(conn)
cluster, err := clusterIf.Get(ctx, &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
errors.CheckError(err)
diffOption.local = local
diffOption.localRepoRoot = localRepoRoot
diffOption.cluster = cluster
} else if local != "" {
if serverSideGenerate {
client, err := appIf.GetManifestsWithFiles(ctx, grpc_retry.Disable())
errors.CheckError(err)

err = manifeststream.SendApplicationManifestQueryWithFiles(ctx, client, appName, local, localIncludes)
errors.CheckError(err)

res, err := client.CloseAndRecv()
errors.CheckError(err)

diffOption.serversideRes = res
} else {
conn, clusterIf := clientset.NewClusterClientOrDie()
notfromstatefarm marked this conversation as resolved.
Show resolved Hide resolved
defer argoio.Close(conn)
cluster, err := clusterIf.Get(ctx, &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
errors.CheckError(err)
diffOption.local = local
diffOption.localRepoRoot = localRepoRoot
diffOption.cluster = cluster
}
}
foundDiffs := findandPrintDiff(ctx, app, resources, argoSettings, appName, diffOption)
if foundDiffs && exitCode {
Expand All @@ -882,6 +898,8 @@ func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
command.Flags().StringVar(&local, "local", "", "Compare live app to a local manifests")
command.Flags().StringVar(&revision, "revision", "", "Compare live app to a particular revision")
command.Flags().StringVar(&localRepoRoot, "local-repo-root", "/", "Path to the repository root. Used together with --local allows setting the repository root")
command.Flags().BoolVar(&serverSideGenerate, "server-side-generate", false, "Used with --local, this will send your manifests to the server for diffing")
command.Flags().StringArrayVar(&localIncludes, "local-include", []string{"*.yaml", "*.yml", "*.json"}, "Used with --server-side-generate, specify patterns of filenames to send")
notfromstatefarm marked this conversation as resolved.
Show resolved Hide resolved
return command
}

Expand All @@ -892,6 +910,7 @@ type DifferenceOption struct {
revision string
cluster *argoappv1.Cluster
res *repoapiclient.ManifestResponse
serversideRes *repoapiclient.ManifestResponse
}

// 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
Expand All @@ -912,6 +931,15 @@ func findandPrintDiff(ctx context.Context, app *argoappv1.Application, resources
}
groupedObjs := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, appName)
} else if diffOptions.serversideRes != nil {
var unstructureds []*unstructured.Unstructured
for _, mfst := range diffOptions.serversideRes.Manifests {
obj, err := argoappv1.UnmarshalToUnstructured(mfst)
errors.CheckError(err)
unstructureds = append(unstructureds, obj)
}
groupedObjs := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, appName)
} else {
for i := range resources.Items {
res := resources.Items[i]
Expand Down
4 changes: 4 additions & 0 deletions docs/operator-manual/argocd-cmd-params-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ data:
# Changing this to "true" will not allow _all_ out-of-bounds symlinks. Those will still be blocked for things like values
# files in Helm charts. But symlinks which are not explicitly blocked by other checks will be allowed.
reposerver.allow.oob.symlinks: "false"
# Maximum size of tarball when streaming manifests to the repo server for generation
reposerver.streamed.manifest.max.tar.size: "10485760"
# Maximum size of extracted manifests when streaming manifests to the repo server for generation
reposerver.streamed.manifest.max.extracted.size: "104857600"
notfromstatefarm marked this conversation as resolved.
Show resolved Hide resolved

# Disable TLS on the HTTP endpoint
dexserver.disable.tls: "false"
2 changes: 2 additions & 0 deletions docs/operator-manual/server-commands/argocd-repo-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ argocd-repo-server [flags]
--revision-cache-expiration duration Cache expiration for cached revision (default 3m0s)
--sentinel stringArray Redis sentinel hostname and port (e.g. argocd-redis-ha-announce-0:6379).
--sentinelmaster string Redis sentinel master group name. (default "master")
--streamed-manifest-max-extracted-size int Maximum size of streamed manifest archives when extracted (default 1048576000)
--streamed-manifest-max-tar-size int Maximum size of streamed manifest archives (default 104857600)
--tlsciphers string The list of acceptable ciphers to be used when establishing TLS connections. Use 'list' to list available ciphers. (default "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_RSA_WITH_AES_256_GCM_SHA384")
--tlsmaxversion string The maximum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.3")
--tlsminversion string The minimum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.2")
Expand Down
16 changes: 9 additions & 7 deletions docs/user-guide/commands/argocd_app_diff.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ argocd app diff APPNAME [flags]
### Options

```
--exit-code Return non-zero exit code when there is a diff (default true)
--hard-refresh Refresh application data as well as target manifests cache
-h, --help help for diff
--local string Compare live app to a local manifests
--local-repo-root string Path to the repository root. Used together with --local allows setting the repository root (default "/")
--refresh Refresh application data when retrieving
--revision string Compare live app to a particular revision
--exit-code Return non-zero exit code when there is a diff (default true)
--hard-refresh Refresh application data as well as target manifests cache
-h, --help help for diff
--local string Compare live app to a local manifests
--local-include stringArray Used with --server-side-generate, specify patterns of filenames to send (default [*.yaml,*.yml,*.json])
--local-repo-root string Path to the repository root. Used together with --local allows setting the repository root (default "/")
--refresh Refresh application data when retrieving
--revision string Compare live app to a particular revision
--server-side-generate Used with --local, this will send your manifests to the server for diffing
```

### Options inherited from parent commands
Expand Down
12 changes: 12 additions & 0 deletions manifests/base/repo-server/argocd-repo-server-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down
12 changes: 12 additions & 0 deletions manifests/core-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9769,6 +9769,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down
12 changes: 12 additions & 0 deletions manifests/ha/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10896,6 +10896,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down
12 changes: 12 additions & 0 deletions manifests/ha/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1670,6 +1670,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down
12 changes: 12 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10238,6 +10238,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down
12 changes: 12 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,18 @@ spec:
key: reposerver.allow.oob.symlinks
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.tar.size
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE
valueFrom:
configMapKeyRef:
key: reposerver.streamed.manifest.max.extracted.size
name: argocd-cmd-params-cm
optional: true
- name: HELM_CACHE_HOME
value: /helm-working-dir
- name: HELM_CONFIG_HOME
Expand Down