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

Enable server-side diffs #314

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/on_pull-request_docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ jobs:
run: ./earthly.sh +rebuild-docs

- name: verify that the checked in file has not changed
run: ./hacks/exit-on-changed-files.sh "Please run './earthly +rebuild-docs' and commit the results to this PR"
run: ./hacks/exit-on-changed-files.sh "Please run './earthly.sh +rebuild-docs' and commit the results to this PR"
4 changes: 4 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ func init() {
newStringOpts().
withDefault("kubechecks again"))

boolFlag(flags, "server-side-diff", "Enable server-side diff.",
newBoolOpts().
withDefault(false))

panicIfError(viper.BindPFlags(flags))
setupLogOutput()
}
Expand Down
1 change: 1 addition & 0 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_REPLAN_COMMENT_MSG`|comment message which re-triggers kubechecks on PR.|`kubechecks again`|
|`KUBECHECKS_REPO_REFRESH_INTERVAL`|Interval between static repo refreshes (for schemas and policies).|`5m`|
|`KUBECHECKS_SCHEMAS_LOCATION`|Sets schema locations to be used for every check request. Can be common paths inside the repos being checked or git urls in either git or http(s) format.|`[]`|
|`KUBECHECKS_SERVER_SIDE_DIFF`|Enable server-side diff.|`false`|
|`KUBECHECKS_SHOW_DEBUG_INFO`|Set to true to print debug info to the footer of MR comments.|`false`|
|`KUBECHECKS_TIDY_OUTDATED_COMMENTS_MODE`|Sets the mode to use when tidying outdated comments. One of hide, delete.|`hide`|
|`KUBECHECKS_VCS_BASE_URL`|VCS base url, useful if self hosting gitlab, enterprise github, etc.||
Expand Down
5 changes: 4 additions & 1 deletion localdev/kubechecks/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ configMap:
KUBECHECKS_NAMESPACE: 'kubechecks'
KUBECHECKS_FALLBACK_K8S_VERSION: "1.25.0"
KUBECHECKS_SHOW_DEBUG_INFO: "true"
KUBECHECKS_VCS_TYPE: "gitlab"
# OTEL
KUBECHECKS_OTEL_COLLECTOR_PORT: "4317"
KUBECHECKS_OTEL_ENABLED: "false"
Expand All @@ -21,14 +22,16 @@ configMap:
# KUBECHECKS_SCHEMAS_LOCATION: https://github.com/zapier/kubecheck-schemas.git
KUBECHECKS_TIDY_OUTDATED_COMMENTS_MODE: "delete"
KUBECHECKS_ENABLE_CONFTEST: "false"
KUBECHECKS_SERVER_SIDE_DIFF: "true"
GRPC_ENFORCE_ALPN_ENABLED: false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting the error credentials: cannot check peer: missing selected ALPN property, hence why I disabled this, for local. I don't know why it was breaking

argoproj/argo-cd#20121



deployment:
annotations:
reloader.stakater.com/auto: "true"

image:
pullPolicy: Never
pullPolicy: IfNotPresent
name: "kubechecks"
tag: ""

Expand Down
44 changes: 44 additions & 0 deletions pkg/argo_client/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
repoapiclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient"
"github.com/argoproj/argo-cd/v2/reposerver/repository"
"github.com/argoproj/argo-cd/v2/util/git"
"github.com/argoproj/argo-cd/v2/util/manifeststream"
"github.com/ghodss/yaml"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
"github.com/pkg/errors"
"github.com/rs/zerolog/log"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -102,6 +104,48 @@ func (argo *ArgoClient) generateManifests(
)
}

// adapted fromm https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L894
func (argo *ArgoClient) GetManifestsServerSide(ctx context.Context, name, tempRepoDir, changedAppFilePath string, app argoappv1.Application) ([]string, error) {
var err error

ctx, span := tracer.Start(ctx, "GetManifestsServerSide")
defer span.End()

log.Debug().Str("name", name).Msg("GetManifestsServerSide")

start := time.Now()
defer func() {
duration := time.Since(start)
getManifestsDuration.WithLabelValues(name).Observe(duration.Seconds())
}()

appCloser, appClient := argo.GetApplicationClient()
defer appCloser.Close()

client, err := appClient.GetManifestsWithFiles(ctx, grpc_retry.Disable())
if err != nil {
return nil, errors.Wrap(err, "failed to get manifest client")
}
localIncludes := []string{"*"}
log.Debug().Str("name", name).Str("repo_path", tempRepoDir).Msg("sending application manifest query with files")

err = manifeststream.SendApplicationManifestQueryWithFiles(ctx, client, name, app.Namespace, tempRepoDir, localIncludes)
if err != nil {
return nil, errors.Wrap(err, "failed to send manifest query")
}

res, err := client.CloseAndRecv()
if err != nil {
return nil, errors.Wrap(err, "failed to receive manifest response")
}

if res.Manifests == nil {
return nil, nil
}
getManifestsSuccess.WithLabelValues(name).Inc()
return res.Manifests, nil
}

func ConvertJsonToYamlManifests(jsonManifests []string) []string {
var manifests []string
for _, manifest := range jsonManifests {
Expand Down
79 changes: 79 additions & 0 deletions pkg/checks/diff/diff_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package diff

import (
"encoding/json"
"testing"

argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
"github.com/stretchr/testify/assert"
)

func TestIsApp(t *testing.T) {
tests := []struct {
name string
item objKeyLiveTarget
manifests []byte
expected bool
}{
{
name: "Valid Application",
item: objKeyLiveTarget{
key: kube.ResourceKey{
Group: "argoproj.io",
Kind: "Application",
},
},
manifests: func() []byte {
app := argoappv1.Application{
Spec: argoappv1.ApplicationSpec{
Project: "default",
},
}
data, _ := json.Marshal(app)
return data
}(),
expected: true,
},
{
name: "Invalid Group",
item: objKeyLiveTarget{
key: kube.ResourceKey{
Group: "invalid.group",
Kind: "Application",
},
},
manifests: []byte{},
expected: false,
},
{
name: "Invalid Kind",
item: objKeyLiveTarget{
key: kube.ResourceKey{
Group: "argoproj.io",
Kind: "InvalidKind",
},
},
manifests: []byte{},
expected: false,
},
{
name: "Invalid JSON",
item: objKeyLiveTarget{
key: kube.ResourceKey{
Group: "argoproj.io",
Kind: "Application",
},
},
manifests: []byte("invalid json"),
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, result := isApp(tt.item, tt.manifests)
assert.Equal(t, tt.expected, result)
})
}
}
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type ServerConfig struct {
MaxQueueSize int64 `mapstructure:"max-queue-size"`
MaxConcurrenctChecks int `mapstructure:"max-concurrenct-checks"`
ReplanCommentMessage string `mapstructure:"replan-comment-msg"`
ServerSideDiff bool `mapstructure:"server-side-diff"`
}

func New() (ServerConfig, error) {
Expand Down Expand Up @@ -115,6 +116,7 @@ func NewWithViper(v *viper.Viper) (ServerConfig, error) {
log.Info().Msgf("Webhook URL Prefix: %s", cfg.UrlPrefix)
log.Info().Msgf("VCS Type: %s", cfg.VcsType)
log.Info().Msgf("ArgoCD Namespace: %s", cfg.ArgoCDNamespace)
log.Info().Msgf("Server-Side Diff: %v", cfg.ServerSideDiff)

return cfg, nil
}
10 changes: 8 additions & 2 deletions pkg/events/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,15 @@ func (w *worker) processApp(ctx context.Context, app v1alpha1.Application) {
return
}
repoPath := repo.Directory
var jsonManifests []string

logger.Debug().Str("repo_path", repoPath).Msg("Getting manifests")
jsonManifests, err := w.ctr.ArgoClient.GetManifestsLocal(ctx, appName, repoPath, appPath, app)
if !w.ctr.Config.ServerSideDiff {
logger.Debug().Str("repo_path", repoPath).Msg("Getting manifests")
jsonManifests, err = w.ctr.ArgoClient.GetManifestsLocal(ctx, appName, repoPath, appPath, app)
} else {
logger.Debug().Str("repo_path", repoPath).Msg("Getting server-side manifests")
jsonManifests, err = w.ctr.ArgoClient.GetManifestsServerSide(ctx, appName, repoPath, appPath, app)
}
if err != nil {
logger.Error().Err(err).Msg("Unable to get manifests")
w.vcsNote.AddToAppMessage(ctx, appName, msg.Result{
Expand Down