From 4132f3195966c917eba80e478f9b01cda90c0d30 Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Fri, 14 Jul 2023 17:17:19 -0400 Subject: [PATCH 1/3] chore: Update log level to warn when in-cluster svr addr is disabled but internal addr is used (#14520) Signed-off-by: Yuan Tang --- util/db/cluster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/db/cluster.go b/util/db/cluster.go index c3d104d89239a..22c92dbde32aa 100644 --- a/util/db/cluster.go +++ b/util/db/cluster.go @@ -78,7 +78,7 @@ func (db *db) ListClusters(ctx context.Context) (*appv1.ClusterList, error) { hasInClusterCredentials = true clusterList.Items = append(clusterList.Items, *cluster) } else { - log.Errorf("failed to add cluster %q to cluster list: in-cluster server address is disabled in Argo CD settings", cluster.Name) + log.Warnf("failed to add cluster %q to cluster list: in-cluster server address is disabled in Argo CD settings", cluster.Name) } } else { clusterList.Items = append(clusterList.Items, *cluster) From 437e9b4961bb1de3d7f449ae42982e88f1302e25 Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Mon, 17 Jul 2023 12:54:49 -0400 Subject: [PATCH 2/3] chore: Print in-cluster svr addr disabled warning during ArgoDB initialization (#14539) * chore: Print in-cluster svr addr disabled warning during ArgoDB initialization Signed-off-by: Yuan Tang * fix: undo a change Signed-off-by: Yuan Tang * chore: move to a function Signed-off-by: Yuan Tang * chore: rename Signed-off-by: Yuan Tang --------- Signed-off-by: Yuan Tang --- util/db/cluster.go | 2 -- util/db/db.go | 29 ++++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/util/db/cluster.go b/util/db/cluster.go index 22c92dbde32aa..d821b59ff6a0e 100644 --- a/util/db/cluster.go +++ b/util/db/cluster.go @@ -77,8 +77,6 @@ func (db *db) ListClusters(ctx context.Context) (*appv1.ClusterList, error) { if inClusterEnabled { hasInClusterCredentials = true clusterList.Items = append(clusterList.Items, *cluster) - } else { - log.Warnf("failed to add cluster %q to cluster list: in-cluster server address is disabled in Argo CD settings", cluster.Name) } } else { clusterList.Items = append(clusterList.Items, *cluster) diff --git a/util/db/db.go b/util/db/db.go index 05ae38e75bb84..f66cf65dc9c47 100644 --- a/util/db/db.go +++ b/util/db/db.go @@ -4,9 +4,11 @@ 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" ) @@ -93,11 +95,36 @@ type db struct { // NewDB returns a new instance of the argo database func NewDB(namespace string, settingsMgr *settings.SettingsManager, kubeclientset kubernetes.Interface) ArgoDB { - return &db{ + dbInstance := db{ settingsMgr: settingsMgr, ns: namespace, kubeclientset: kubeclientset, } + dbInstance.logInClusterWarning() + return &dbInstance +} + +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) { From d28bfff61eba98e9358ccb3272e3616a39070f35 Mon Sep 17 00:00:00 2001 From: Yuan Tang Date: Mon, 24 Jul 2023 13:49:51 -0400 Subject: [PATCH 3/3] chore: Print in-cluster svr addr disabled warning when server starts (#14553) * chore: Print in-cluster svr addr disabled warning when server starts Signed-off-by: Yuan Tang * fix: mock Signed-off-by: Yuan Tang * no interface change Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Yuan Tang Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- server/server.go | 57 +++++++++++++++++++++++++++++++++++++++-- util/db/cluster.go | 20 +++++++-------- util/db/cluster_test.go | 6 ++--- util/db/db.go | 29 +-------------------- 4 files changed, 69 insertions(+), 43 deletions(-) diff --git a/server/server.go b/server/server.go index 50733d5ce2ed3..fdce390701c4c 100644 --- a/server/server.go +++ b/server/server.go @@ -24,6 +24,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" @@ -287,7 +289,9 @@ 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) - return &ArgoCDServer{ + dbInstance := db.NewDB(opts.Namespace, settingsMgr, opts.KubeClientset) + + a := &ArgoCDServer{ ArgoCDServerOpts: opts, log: log.NewEntry(log.StandardLogger()), settings: settings, @@ -303,11 +307,19 @@ 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, } + + 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 ( @@ -354,6 +366,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 d821b59ff6a0e..cfc546ef87028 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, 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 f66cf65dc9c47..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" ) @@ -95,36 +93,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, } - dbInstance.logInClusterWarning() - return &dbInstance -} - -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) {