Compass Runtime Agent should reconcile the mTLS secret in istio-system (kyma-project#12626)

* Checking secret existence in controller's reconcilation loop.

* Fix for failing unit tests

* Image bumped

* adding one unit test to check credential manager

* extend logging with new flag CreadentialsExist

* Regenerating mock for Connector interface for modified MaintainConnection method

* fix for saving reneved credentials

* fix for saving reneved credentials #2

* remove useless comment

Co-authored-by: Przemyslaw Golicz <[email protected]>

Author: akgalwas

components/compass-runtime-agent/internal/certificates/manager.go

@@ -22,6 +22,7 @@ const (
 type Manager interface {
 	GetClientCredentials() (ClientCredentials, error)
 	PreserveCredentials(Credentials) error
+	CredentialsExist() (bool, error)
 }
 
 func NewCredentialsManager(clusterCertificateSecretName, caCertSecretName types.NamespacedName, secretsRepository secrets.Repository) *credentialsManager {
@@ -53,6 +54,10 @@ func (cm *credentialsManager) GetClientCredentials() (ClientCredentials, error)
 	return pemCredentials.AsClientCredentials()
 }
 
+func (cm *credentialsManager) CredentialsExist() (bool, error) {
+	return cm.secretsRepository.Exists(cm.caCertSecretName)
+}
+
 func (cm *credentialsManager) PreserveCredentials(credentials Credentials) error {
 	pemCredentials := credentials.AsPemEncoded()
 

components/compass-runtime-agent/internal/certificates/manager_test.go

@@ -31,6 +31,38 @@ var (
 	}
 )
 
+func TestCredentialsExist(t *testing.T) {
+	t.Run("should return false if credentials does not exist", func(t *testing.T) {
+		// given
+		expectedErr := errors.New("oh, no")
+
+		secretsRepository := &mocks.Repository{}
+		secretsRepository.On("Exists", caCertSecretNamespaceName).Return(false, expectedErr)
+
+		// when
+		credentialsManager := NewCredentialsManager(clusterCertSecretNamespaceName, caCertSecretNamespaceName, secretsRepository)
+
+		// then
+		exists, err := credentialsManager.CredentialsExist()
+		assert.Equal(t, expectedErr, err)
+		assert.Equal(t, false, exists)
+	})
+
+	t.Run("should return true if credentials exist", func(t *testing.T) {
+		// given
+		secretsRepository := &mocks.Repository{}
+		secretsRepository.On("Exists", caCertSecretNamespaceName).Return(true, nil)
+
+		// when
+		credentialsManager := NewCredentialsManager(clusterCertSecretNamespaceName, caCertSecretNamespaceName, secretsRepository)
+
+		// then
+		exists, err := credentialsManager.CredentialsExist()
+		assert.Equal(t, nil, err)
+		assert.Equal(t, true, exists)
+	})
+}
+
 func TestCertificatePreserver_PreserveCertificates(t *testing.T) {
 
 	pemCredentials := PemEncodedCredentials{

components/compass-runtime-agent/internal/certificates/mocks/Manager.go

@@ -26,7 +26,7 @@ const (
 //go:generate mockery --name=Connector
 type Connector interface {
 	EstablishConnection(connectorURL, token string) (EstablishedConnection, error)
-	MaintainConnection(renewCert bool) (*certificates.Credentials, v1alpha1.ManagementInfo, error)
+	MaintainConnection(renewCert bool, credentialsExist bool) (*certificates.Credentials, v1alpha1.ManagementInfo, error)
 }
 
 func NewCompassConnector(
@@ -91,7 +91,7 @@ func (cc *compassConnector) establishConnection(connectorURL, token, requestID s
 	}, nil
 }
 
-func (cc *compassConnector) MaintainConnection(renewCert bool) (*certificates.Credentials, v1alpha1.ManagementInfo, error) {
+func (cc *compassConnector) MaintainConnection(renewCert bool, credentialsExist bool) (*certificates.Credentials, v1alpha1.ManagementInfo, error) {
 	certSecuredClient, err := cc.clientsProvider.GetConnectorCertSecuredClient()
 	if err != nil {
 		return nil, v1alpha1.ManagementInfo{}, errors.Wrap(err, "Failed to prepare Certificate-secured Connector client while checking connection")
@@ -107,7 +107,7 @@ func (cc *compassConnector) MaintainConnection(renewCert bool) (*certificates.Cr
 		return nil, v1alpha1.ManagementInfo{}, err
 	}
 
-	if !renewCert {
+	if !renewCert && credentialsExist {
 		return nil, toManagementInfo(configuration.ManagementPlaneInfo), nil
 	}
 

components/compass-runtime-agent/internal/compassconnection/connector.go

@@ -123,6 +123,8 @@ func TestCompassConnectionController(t *testing.T) {
 		assert.True(t, ok)
 		assert.NotEmpty(t, credentials)
 	}).Return(nil)
+	credentialsManagerMock.On("CredentialsExist").Return(true, nil)
+
 	// Config provider
 	configProviderMock := configProviderMock()
 	// Connector clients

components/compass-runtime-agent/internal/compassconnection/mocks/Connector.go

@@ -118,7 +118,6 @@ func (s *crSupervisor) InitializeCompassConnection() (*v1alpha1.CompassConnectio
 	return s.updateCompassConnection(compassConnectionCR)
 }
 
-// SynchronizeWithCompass synchronizes with Compass
 func (s *crSupervisor) SynchronizeWithCompass(connection *v1alpha1.CompassConnection) (*v1alpha1.CompassConnection, error) {
 	s.log = s.log.WithField("CompassConnection", connection.Name)
 
@@ -194,16 +193,20 @@ func (s *crSupervisor) SynchronizeWithCompass(connection *v1alpha1.CompassConnec
 
 func (s *crSupervisor) maintainCompassConnection(compassConnection *v1alpha1.CompassConnection) error {
 	shouldRenew := compassConnection.ShouldRenewCertificate(s.certValidityRenewalThreshold, s.minimalCompassSyncTime)
+	credentialsExist, err := s.credentialsManager.CredentialsExist()
+	if err != nil {
+		return errors.Wrap(err, "Failed to check whether credentials exist")
+	}
 
-	s.log.Infof("Trying to maintain certificates connection... Renewal: %v", shouldRenew)
-	newCreds, managementInfo, err := s.compassConnector.MaintainConnection(shouldRenew)
+	s.log.Infof("Trying to maintain certificates connection... Renewal: %v, CreadentialsExist: %v", shouldRenew, credentialsExist)
+	newCreds, managementInfo, err := s.compassConnector.MaintainConnection(shouldRenew, credentialsExist)
 	if err != nil {
 		return errors.Wrap(err, "Failed to connect to Compass Connector")
 	}
 
 	connectionTime := metav1.Now()
 
-	if shouldRenew && newCreds != nil {
+	if newCreds != nil {
 		s.log.Infof("Trying to save renewed certificates...")
 		err = s.credentialsManager.PreserveCredentials(*newCreds)
 		if err != nil {

components/compass-runtime-agent/internal/compassconnection/package_test.go

@@ -27,6 +27,7 @@ type Manager interface {
 //go:generate mockery --name=Repository
 // Repository contains operations for managing client credentials
 type Repository interface {
+	Exists(name types.NamespacedName) (bool, error)
 	Get(name types.NamespacedName) (map[string][]byte, error)
 	UpsertWithReplace(name types.NamespacedName, data map[string][]byte) error
 	UpsertWithMerge(name types.NamespacedName, data map[string][]byte) error
@@ -45,6 +46,20 @@ func NewRepository(secretsManagerConstructor ManagerConstructor) Repository {
 	}
 }
 
+func (r *repository) Exists(name types.NamespacedName) (bool, error) {
+	secretManager := r.secretsManagerConstructor(name.Namespace)
+
+	_, err := secretManager.Get(context.Background(), name.Name, metav1.GetOptions{})
+	if err != nil {
+		if k8serrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+
+	return true, nil
+}
+
 // UpsertWithReplace creates a new Kubernetes secret, if secret with specified name already exists overrides it
 func (r *repository) UpsertWithReplace(name types.NamespacedName, data map[string][]byte) error {
 	secretManager := r.secretsManagerConstructor(name.Namespace)

components/compass-runtime-agent/internal/compassconnection/supervisor.go

@@ -35,6 +35,55 @@ var (
 	}
 )
 
+func TestRepository_Exists(t *testing.T) {
+	t.Run("should return true if exists", func(t *testing.T) {
+		// given
+		secret := makeSecret(namespacedName, map[string][]byte{dataKey: []byte("data")})
+
+		secretsManagerMock := &mocks.Manager{}
+		secretsManagerMock.On("Get", context.Background(), secretName, metav1.GetOptions{}).Return(secret, nil)
+
+		repository := NewRepository(prepareManagerConstructor(secretsManagerMock))
+
+		// when
+		exists, err := repository.Exists(namespacedName)
+
+		// then
+		assert.NoError(t, err)
+		assert.Equal(t, true, exists)
+	})
+
+	t.Run("should return false if secret doesn't exist", func(t *testing.T) {
+		// given
+		secretsManagerMock := &mocks.Manager{}
+		secretsManagerMock.On("Get", context.Background(), secretName, metav1.GetOptions{}).Return(nil, k8serrors.NewNotFound(schema.GroupResource{}, "secret"))
+
+		repository := NewRepository(prepareManagerConstructor(secretsManagerMock))
+
+		// when
+		exists, err := repository.Exists(namespacedName)
+
+		// then
+		assert.NoError(t, err)
+		assert.Equal(t, false, exists)
+	})
+
+	t.Run("should return error if failed to read secret", func(t *testing.T) {
+		// given
+		secretsManagerMock := &mocks.Manager{}
+		secretsManagerMock.On("Get", context.Background(), secretName, metav1.GetOptions{}).Return(nil, errors.New("oh, no"))
+
+		repository := NewRepository(prepareManagerConstructor(secretsManagerMock))
+
+		// when
+		exists, err := repository.Exists(namespacedName)
+
+		// then
+		assert.Error(t, err)
+		assert.Equal(t, false, exists)
+	})
+}
+
 func TestRepository_Get(t *testing.T) {
 	t.Run("should get given secret", func(t *testing.T) {
 		// given