chore: Print in-cluster svr addr disabled warning when server starts (a…

…rgoproj#14553) * chore: Print in-cluster svr addr disabled warning when server starts Signed-off-by: Yuan Tang <[email protected]> * fix: mock Signed-off-by: Yuan Tang <[email protected]> * no interface change Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Yuan Tang <[email protected]> Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]>
terrytangyuan · Jul 24, 2023 · 07bb664 · 07bb664
1 parent 3dee626
commit 07bb664
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 43 deletions.
diff --git 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"
@@ -291,7 +293,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,
@@ -307,11 +311,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 (
@@ -358,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
@@ -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
@@ -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
@@ -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) {