From 5c9466ed12479e68c5dbabdebc63419a848c8a3d Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Mon, 17 Jul 2023 13:47:39 -0400 Subject: [PATCH 1/3] chore: Print in-cluster svr addr disabled warning when server starts Signed-off-by: Yuan Tang --- server/server.go | 4 +++- util/db/db.go | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/server/server.go b/server/server.go index f0f2df6680ad7..a46e001ae3761 100644 --- a/server/server.go +++ b/server/server.go @@ -291,6 +291,8 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer { apiFactory := api.NewFactory(settings_notif.GetFactorySettings(argocdService, "argocd-notifications-secret", "argocd-notifications-cm"), opts.Namespace, secretInformer, configMapInformer) + dbInstance := db.NewDB(opts.Namespace, settingsMgr, opts.KubeClientset) + dbInstance.LogInClusterWarning() return &ArgoCDServer{ ArgoCDServerOpts: opts, log: log.NewEntry(log.StandardLogger()), @@ -307,7 +309,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer { policyEnforcer: policyEnf, userStateStorage: userStateStorage, staticAssets: http.FS(staticFS), - db: db.NewDB(opts.Namespace, settingsMgr, opts.KubeClientset), + db: dbInstance, apiFactory: apiFactory, secretInformer: secretInformer, configMapInformer: configMapInformer, diff --git a/util/db/db.go b/util/db/db.go index f66cf65dc9c47..83c6b197efa6c 100644 --- a/util/db/db.go +++ b/util/db/db.go @@ -85,6 +85,8 @@ type ArgoDB interface { AddGPGPublicKey(ctx context.Context, keyData string) (map[string]*appv1.GnuPGPublicKey, []string, error) // DeleteGPGPublicKey removes a GPG public key from the configuration DeleteGPGPublicKey(ctx context.Context, keyID string) error + // LogInClusterWarning checks the in-cluster configuration and prints out any warnings. + LogInClusterWarning() } type db struct { @@ -100,11 +102,11 @@ func NewDB(namespace string, settingsMgr *settings.SettingsManager, kubeclientse ns: namespace, kubeclientset: kubeclientset, } - dbInstance.logInClusterWarning() return &dbInstance } -func (db *db) logInClusterWarning() { +// LogInClusterWarning checks the in-cluster configuration and prints out any warnings. +func (db *db) LogInClusterWarning() { clusterSecrets, err := db.listSecretsByType(common.LabelValueSecretTypeCluster) if err != nil { log.WithError(err).Errorln("could not list secrets by type") From ffd18c9c776e6119d52058b1668307237966e0ee Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Mon, 17 Jul 2023 20:47:00 -0400 Subject: [PATCH 2/3] fix: mock Signed-off-by: Yuan Tang --- util/db/db.go | 3 +-- util/db/mocks/ArgoDB.go | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/util/db/db.go b/util/db/db.go index 83c6b197efa6c..0feb9ae13238e 100644 --- a/util/db/db.go +++ b/util/db/db.go @@ -97,12 +97,11 @@ type db struct { // NewDB returns a new instance of the argo database func NewDB(namespace string, settingsMgr *settings.SettingsManager, kubeclientset kubernetes.Interface) ArgoDB { - dbInstance := db{ + return &db{ settingsMgr: settingsMgr, ns: namespace, kubeclientset: kubeclientset, } - return &dbInstance } // LogInClusterWarning checks the in-cluster configuration and prints out any warnings. diff --git a/util/db/mocks/ArgoDB.go b/util/db/mocks/ArgoDB.go index eed84975d9080..1d84ecc579c4a 100644 --- a/util/db/mocks/ArgoDB.go +++ b/util/db/mocks/ArgoDB.go @@ -621,3 +621,8 @@ func (_m *ArgoDB) WatchClusters(ctx context.Context, handleAddEvent func(*v1alph return r0 } + +// LogInClusterWarning checks the in-cluster configuration and prints out any warnings. +func (_m *ArgoDB) LogInClusterWarning() { + return +} From 08b91d5acd15959deca0fadaab12ad749d430bed Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Tue, 18 Jul 2023 14:17:47 -0400 Subject: [PATCH 3/3] no interface change Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- server/server.go | 55 +++++++++++++++++++++++++++++++++++++++-- util/db/cluster.go | 20 +++++++-------- util/db/cluster_test.go | 6 ++--- util/db/db.go | 28 --------------------- util/db/mocks/ArgoDB.go | 5 ---- 5 files changed, 66 insertions(+), 48 deletions(-) diff --git a/server/server.go b/server/server.go index a46e001ae3761..7b36d9bca9861 100644 --- a/server/server.go +++ b/server/server.go @@ -25,6 +25,8 @@ import ( // nolint:staticcheck golang_proto "github.com/golang/protobuf/proto" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "github.com/argoproj/notifications-engine/pkg/api" "github.com/argoproj/pkg/sync" @@ -292,8 +294,8 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer { apiFactory := api.NewFactory(settings_notif.GetFactorySettings(argocdService, "argocd-notifications-secret", "argocd-notifications-cm"), opts.Namespace, secretInformer, configMapInformer) dbInstance := db.NewDB(opts.Namespace, settingsMgr, opts.KubeClientset) - dbInstance.LogInClusterWarning() - return &ArgoCDServer{ + + a := &ArgoCDServer{ ArgoCDServerOpts: opts, log: log.NewEntry(log.StandardLogger()), settings: settings, @@ -314,6 +316,14 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer { secretInformer: secretInformer, configMapInformer: configMapInformer, } + + err = a.logInClusterWarnings() + if err != nil { + // Just log. It's not critical. + log.Warnf("Failed to log in-cluster warnings: %v", err) + } + + return a } const ( @@ -360,6 +370,47 @@ func (l *Listeners) Close() error { return nil } +// logInClusterWarnings checks the in-cluster configuration and prints out any warnings. +func (a *ArgoCDServer) logInClusterWarnings() error { + labelSelector := labels.NewSelector() + req, err := labels.NewRequirement(common.LabelKeySecretType, selection.Equals, []string{common.LabelValueSecretTypeCluster}) + if err != nil { + return fmt.Errorf("failed to construct cluster-type label selector: %w", err) + } + labelSelector = labelSelector.Add(*req) + secretsLister, err := a.settingsMgr.GetSecretsLister() + if err != nil { + return fmt.Errorf("failed to get secrets lister: %w", err) + } + clusterSecrets, err := secretsLister.Secrets(a.ArgoCDServerOpts.Namespace).List(labelSelector) + if err != nil { + return fmt.Errorf("failed to list cluster secrets: %w", err) + } + var inClusterSecrets []string + for _, clusterSecret := range clusterSecrets { + cluster, err := db.SecretToCluster(clusterSecret) + if err != nil { + return fmt.Errorf("could not unmarshal cluster secret %q: %w", clusterSecret.Name, err) + } + if cluster.Server == v1alpha1.KubernetesInternalAPIServerAddr { + inClusterSecrets = append(inClusterSecrets, clusterSecret.Name) + } + } + if len(inClusterSecrets) > 0 { + // Don't make this call unless we actually have in-cluster secrets, to save time. + dbSettings, err := a.settingsMgr.GetSettings() + if err != nil { + return fmt.Errorf("could not get DB settings: %w", err) + } + if !dbSettings.InClusterEnabled { + for _, clusterName := range inClusterSecrets { + log.Warnf("cluster %q uses in-cluster server address but it's disabled in Argo CD settings", clusterName) + } + } + } + return nil +} + func startListener(host string, port int) (net.Listener, error) { var conn net.Listener var realErr error diff --git a/util/db/cluster.go b/util/db/cluster.go index df1644e0dbbb9..9b405a9cacd60 100644 --- a/util/db/cluster.go +++ b/util/db/cluster.go @@ -68,7 +68,7 @@ func (db *db) ListClusters(ctx context.Context) (*appv1.ClusterList, error) { inClusterEnabled := settings.InClusterEnabled hasInClusterCredentials := false for _, clusterSecret := range clusterSecrets { - cluster, err := secretToCluster(clusterSecret) + cluster, err := SecretToCluster(clusterSecret) if err != nil { log.Errorf("could not unmarshal cluster secret %s", clusterSecret.Name) continue @@ -120,7 +120,7 @@ func (db *db) CreateCluster(ctx context.Context, c *appv1.Cluster) (*appv1.Clust return nil, err } - cluster, err := secretToCluster(clusterSecret) + cluster, err := SecretToCluster(clusterSecret) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "could not unmarshal cluster secret %s", clusterSecret.Name) } @@ -148,7 +148,7 @@ func (db *db) WatchClusters(ctx context.Context, common.LabelValueSecretTypeCluster, func(secret *apiv1.Secret) { - cluster, err := secretToCluster(secret) + cluster, err := SecretToCluster(secret) if err != nil { log.Errorf("could not unmarshal cluster secret %s", secret.Name) return @@ -163,12 +163,12 @@ func (db *db) WatchClusters(ctx context.Context, }, func(oldSecret *apiv1.Secret, newSecret *apiv1.Secret) { - oldCluster, err := secretToCluster(oldSecret) + oldCluster, err := SecretToCluster(oldSecret) if err != nil { log.Errorf("could not unmarshal cluster secret %s", oldSecret.Name) return } - newCluster, err := secretToCluster(newSecret) + newCluster, err := SecretToCluster(newSecret) if err != nil { log.Errorf("could not unmarshal cluster secret %s", newSecret.Name) return @@ -218,7 +218,7 @@ func (db *db) GetCluster(_ context.Context, server string) (*appv1.Cluster, erro return nil, err } if len(res) > 0 { - return secretToCluster(res[0].(*apiv1.Secret)) + return SecretToCluster(res[0].(*apiv1.Secret)) } if server == appv1.KubernetesInternalAPIServerAddr { return db.getLocalCluster(), nil @@ -239,7 +239,7 @@ func (db *db) GetProjectClusters(ctx context.Context, project string) ([]*appv1. } var res []*appv1.Cluster for i := range secrets { - cluster, err := secretToCluster(secrets[i].(*apiv1.Secret)) + cluster, err := SecretToCluster(secrets[i].(*apiv1.Secret)) if err != nil { return nil, fmt.Errorf("failed to convert secret to cluster: %w", err) } @@ -293,7 +293,7 @@ func (db *db) UpdateCluster(ctx context.Context, c *appv1.Cluster) (*appv1.Clust if err != nil { return nil, err } - cluster, err := secretToCluster(clusterSecret) + cluster, err := SecretToCluster(clusterSecret) if err != nil { log.Errorf("could not unmarshal cluster secret %s", clusterSecret.Name) return nil, err @@ -360,8 +360,8 @@ func clusterToSecret(c *appv1.Cluster, secret *apiv1.Secret) error { return nil } -// secretToCluster converts a secret into a Cluster object -func secretToCluster(s *apiv1.Secret) (*appv1.Cluster, error) { +// SecretToCluster converts a secret into a Cluster object +func SecretToCluster(s *apiv1.Secret) (*appv1.Cluster, error) { var config appv1.ClusterConfig if len(s.Data["config"]) > 0 { err := json.Unmarshal(s.Data["config"], &config) diff --git a/util/db/cluster_test.go b/util/db/cluster_test.go index c3b273b4fe5ef..9d60a3073c3c2 100644 --- a/util/db/cluster_test.go +++ b/util/db/cluster_test.go @@ -43,7 +43,7 @@ func Test_secretToCluster(t *testing.T) { "config": []byte("{\"username\":\"foo\"}"), }, } - cluster, err := secretToCluster(secret) + cluster, err := SecretToCluster(secret) require.NoError(t, err) assert.Equal(t, *cluster, v1alpha1.Cluster{ Name: "test", @@ -89,7 +89,7 @@ func Test_secretToCluster_NoConfig(t *testing.T) { "server": []byte("http://mycluster"), }, } - cluster, err := secretToCluster(secret) + cluster, err := SecretToCluster(secret) assert.NoError(t, err) assert.Equal(t, *cluster, v1alpha1.Cluster{ Name: "test", @@ -111,7 +111,7 @@ func Test_secretToCluster_InvalidConfig(t *testing.T) { "config": []byte("{'tlsClientConfig':{'insecure':false}}"), }, } - cluster, err := secretToCluster(secret) + cluster, err := SecretToCluster(secret) require.Error(t, err) assert.Nil(t, cluster) } diff --git a/util/db/db.go b/util/db/db.go index 0feb9ae13238e..05ae38e75bb84 100644 --- a/util/db/db.go +++ b/util/db/db.go @@ -4,11 +4,9 @@ import ( "context" "strings" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" - "github.com/argoproj/argo-cd/v2/common" appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/util/settings" ) @@ -85,8 +83,6 @@ type ArgoDB interface { AddGPGPublicKey(ctx context.Context, keyData string) (map[string]*appv1.GnuPGPublicKey, []string, error) // DeleteGPGPublicKey removes a GPG public key from the configuration DeleteGPGPublicKey(ctx context.Context, keyID string) error - // LogInClusterWarning checks the in-cluster configuration and prints out any warnings. - LogInClusterWarning() } type db struct { @@ -104,30 +100,6 @@ func NewDB(namespace string, settingsMgr *settings.SettingsManager, kubeclientse } } -// LogInClusterWarning checks the in-cluster configuration and prints out any warnings. -func (db *db) LogInClusterWarning() { - clusterSecrets, err := db.listSecretsByType(common.LabelValueSecretTypeCluster) - if err != nil { - log.WithError(err).Errorln("could not list secrets by type") - } - dbSettings, err := db.settingsMgr.GetSettings() - if err != nil { - log.WithError(err).Errorln("could not get DB settings") - } - for _, clusterSecret := range clusterSecrets { - cluster, err := secretToCluster(clusterSecret) - if err != nil { - log.Errorf("could not unmarshal cluster secret %s", clusterSecret.Name) - continue - } - if cluster.Server == appv1.KubernetesInternalAPIServerAddr { - if !dbSettings.InClusterEnabled { - log.Warnf("cluster %q uses in-cluster server address but it's disabled in Argo CD settings", cluster.Name) - } - } - } -} - func (db *db) getSecret(name string, cache map[string]*v1.Secret) (*v1.Secret, error) { secret, ok := cache[name] if !ok { diff --git a/util/db/mocks/ArgoDB.go b/util/db/mocks/ArgoDB.go index 1d84ecc579c4a..eed84975d9080 100644 --- a/util/db/mocks/ArgoDB.go +++ b/util/db/mocks/ArgoDB.go @@ -621,8 +621,3 @@ func (_m *ArgoDB) WatchClusters(ctx context.Context, handleAddEvent func(*v1alph return r0 } - -// LogInClusterWarning checks the in-cluster configuration and prints out any warnings. -func (_m *ArgoDB) LogInClusterWarning() { - return -}