Changes to report coverage to ODL+ test fixes+ catering to review (#249)

* Changes to report coverage to ODL+ test fixes+ catering to review

* Removing codecov force flag

.gitignore

@@ -6,3 +6,4 @@ out*
 *.out
 istio-*
 .DS_Store
+cobertura-coverage.xml

Jenkinsfile.iks2

@@ -9,7 +9,11 @@ def dockerRegistryCredsId = 'service-mesh-docker-cred'
 def ptNameVersion = "admiral-${UUID.randomUUID().toString().toLowerCase()}"
 
 def jenkins_pod_label = "admiral-podman-build-pod-label"
-
+def admiral_codecov_token = [
+        $class: 'StringBinding',
+        credentialsId: 'admiral-codecov-token',
+        variable: 'CODECOV_TOKEN'
+]
 commitSha = ''
 
 envMap = [:]
@@ -120,14 +124,40 @@ pipeline {
                 {
                   sh "cd /workspace/go/src/github.com/istio-ecosystem/admiral"
                   sh "make build"
+                  sh "make test"
+                  withCredentials([admiral_codecov_token]){
+                        output = sh(script: "curl -s https://codecov.tools.a.intuit.com/bash | bash -s - -t ${CODECOV_TOKEN} -f c.out", returnStdout: true).trim()
+                        echo "output from codecov: ${output}"
+                  }
                   admiralTag = IMAGE_TAG
+                  sh "ls -la"
                 }
               }
             }
           }
         }
       }
 
+      stage('Cobertura') {
+           steps {
+                cobertura coberturaReportFile: "cobertura-coverage.xml"
+           }
+      }
+
+      stage('Publish Build Metrics') {
+             steps {
+                      buildMetrics artifactsOption: true,
+                      boxsetOption: true,
+                      assetId: "8287766806579881856",
+                      preSetArtifacts: [[
+                           type: 'json',
+                           artifactId: 'admiral',
+                           groupId: 'com.intuit.devx',
+                           version: "${env.GIT_COMMIT.toString()}"
+                       ]]
+            }
+     }
+
       stage('push') {
           steps {
             script {

Makefile

@@ -12,6 +12,8 @@ GOBUILD?=$(GOCMD) build
 GOCLEAN?=$(GOCMD) clean
 GOTEST?=$(GOCMD) test -race
 GOGET?=$(GOCMD) get
+GOPATHENTRY:=$(shell $(GOCMD) env | grep GOPATH | grep -o '"[^=]\+$"')
+GOPATH=$(subst $\",,$(GOPATHENTRY))
 GOBIN?=$(GOPATH)/bin
 OUT?=./out/
 
@@ -46,6 +48,8 @@ build-mac:
 
 test:
 	$(GOTEST) -v -race `go list ./... | grep -v client` -coverprofile=c.out
+	$(GOCMD) install github.com/boumenot/gocover-cobertura@latest
+	$(GOPATH)/bin/gocover-cobertura < c.out > cobertura-coverage.xml
 
 clean:
 	$(GOCLEAN)

admiral/pkg/client/idps.go

@@ -6,14 +6,16 @@ import (
 	"strconv"
 
 	v1 "github.com/istio-ecosystem/admiral/admiral/apis/v1"
-	idps_sdk "github.intuit.com/idps/idps-go-sdk/v3/idps-sdk"
 	"github.intuit.com/idps/idps-go-sdk/v3/idps-sdk/config"
 	"github.intuit.com/idps/idps-go-sdk/v3/idps-sdk/items"
 )
 
+type IdpsClientInterface interface {
+	GetSecret(variables *items.SecretVariables, version int32) (*items.Secret, error)
+}
+
 type IdpsClient struct {
-	idpsClient     *idps_sdk.IdpsClient
-	idpsMgmtClient *idps_sdk.IdpsMgmtClient
+	idpsClient IdpsClientInterface
 }
 
 type IdpsClientAPI interface {
@@ -33,7 +35,11 @@ const (
 	secretName         = "secrets/management/pem"
 )
 
-func NewIdpsClient(serverConfig *v1.AdmiralConfig) (*IdpsClient, error) {
+type IdpsSdkProvider interface {
+	IdpsClientInstanceFromMap(props map[string]string) (IdpsClientInterface, error)
+}
+
+func NewIdpsClient(serverConfig *v1.AdmiralConfig, provider IdpsSdkProvider) (*IdpsClient, error) {
 
 	props := make(map[string]string)
 
@@ -50,7 +56,10 @@ func NewIdpsClient(serverConfig *v1.AdmiralConfig) (*IdpsClient, error) {
 	client := IdpsClient{}
 
 	var err error
-	client.idpsClient, err = idps_sdk.IdpsClientInstanceFromMap(props)
+	if provider == nil {
+		return nil, fmt.Errorf("provider should not be nil")
+	}
+	client.idpsClient, err = provider.IdpsClientInstanceFromMap(props)
 	if err != nil {
 		return nil, err
 	}
@@ -60,12 +69,6 @@ func NewIdpsClient(serverConfig *v1.AdmiralConfig) (*IdpsClient, error) {
 		return nil, err
 	}
 	props[config.MGMT_KEY_CONTENT] = string(key)
-
-	client.idpsMgmtClient, err = idps_sdk.IdpsMgmtClientInstanceFromMap(props)
-	if err != nil {
-		return nil, err
-	}
-
 	return &client, nil
 }
 

admiral/pkg/clusters/admiralIgnoreIdentityStateChecker_test.go

@@ -250,6 +250,8 @@ func TestIgnoreIdentityStateCheckerRunStateCheck(t *testing.T) {
 			max := 10
 			for count <= max {
 				time.Sleep(1 * time.Millisecond)
+				checker.stateCache.RWLock.RLock()
+				defer checker.stateCache.RWLock.RUnlock()
 				if checker.stateCache.Enabled != c.expectedCurrentIgnoreIdentityState.Enabled {
 					if count == max {
 						t.Errorf("expected state cache.Enabled to be: %v, got: %v",

admiral/pkg/clusters/registry.go

@@ -26,6 +26,12 @@ const (
 	LogErrFormat = "op=%s type=%v Name=%v cluster=%s, e=%v"
 )
 
+type KubernetesConfigWrapper struct{}
+
+func (c *KubernetesConfigWrapper) InClusterConfig() (*rest.Config, error) {
+	return rest.InClusterConfig()
+}
+
 func InitAdmiral(ctx context.Context, params common.AdmiralParams) (*RemoteRegistry, error) {
 	log.Infof("Initializing Admiral with params: %v", params)
 	common.InitializeConfig(params)
@@ -62,7 +68,8 @@ func InitAdmiral(ctx context.Context, params common.AdmiralParams) (*RemoteRegis
 		log.Info("argo rollouts disabled")
 	}
 
-	configMapController, err := admiral.NewConfigMapController(params.ServiceEntryIPPrefix)
+	inClusterConfig := &KubernetesConfigWrapper{}
+	configMapController, err := admiral.NewConfigMapController(params.ServiceEntryIPPrefix, inClusterConfig)
 	if err != nil {
 		return nil, fmt.Errorf("error with configmap controller init: %v", err)
 	}

admiral/pkg/controller/admiral/configmap.go

@@ -28,12 +28,16 @@ type ConfigMapController struct {
 
 //todo this is a temp state, eventually changes will have to be made to give each cluster it's own configmap
 
-func NewConfigMapController(seIPPrefix string) (*ConfigMapController, error) {
+type KubernetesConfig interface {
+	InClusterConfig() (*rest.Config, error)
+}
+
+func NewConfigMapController(seIPPrefix string, kc KubernetesConfig) (*ConfigMapController, error) {
 	kubeconfigPath := common.GetKubeconfigPath()
 	namespaceToUse := common.GetSyncNamespace()
 
 	if kubeconfigPath == "" {
-		config, err := rest.InClusterConfig()
+		config, err := kc.InClusterConfig()
 		if err != nil {
 			return nil, err
 		}

admiral/pkg/controller/admiral/configmap_test.go

@@ -3,6 +3,7 @@ package admiral
 import (
 	"context"
 	"errors"
+	"k8s.io/client-go/rest"
 	"testing"
 	"time"
 
@@ -26,6 +27,7 @@ func init() {
 		CacheRefreshDuration:       time.Minute,
 		ClusterRegistriesNamespace: "default",
 		DependenciesNamespace:      "default",
+		Profile:                    common.AdmiralProfileDefault,
 	}
 
 	p.LabelSet.WorkloadIdentityKey = "identity"
@@ -101,6 +103,12 @@ func TestConfigMapController_GetConfigMap(t *testing.T) {
 	}
 }
 
+type MockKubernetesConfig struct{}
+
+func (c *MockKubernetesConfig) InClusterConfig() (*rest.Config, error) {
+	return nil, rest.ErrNotInCluster
+}
+
 func TestNewConfigMapController(t *testing.T) {
 	testCases := []struct {
 		name           string
@@ -124,8 +132,28 @@ func TestNewConfigMapController(t *testing.T) {
 
 	for _, c := range testCases {
 		t.Run(c.name, func(t *testing.T) {
+			common.ResetSync()
+			p := common.AdmiralParams{
+				KubeconfigPath:             "testdata/fake.config",
+				LabelSet:                   &common.LabelSet{},
+				EnableSAN:                  true,
+				SANPrefix:                  "prefix",
+				HostnameSuffix:             "mesh",
+				SyncNamespace:              "ns",
+				CacheRefreshDuration:       time.Minute,
+				ClusterRegistriesNamespace: "default",
+				DependenciesNamespace:      "default",
+				Profile:                    common.AdmiralProfileDefault,
+			}
+
+			p.LabelSet.WorkloadIdentityKey = "identity"
+			p.LabelSet.GlobalTrafficDeploymentLabel = "identity"
+
+			p.LabelSet.EnvKey = "admiral.io/env"
+			common.InitializeConfig(p)
 			common.SetKubeconfigPath(c.kubeconfigPath)
-			controller, err := NewConfigMapController("240.0")
+			mockConfig := &MockKubernetesConfig{}
+			controller, err := NewConfigMapController("240.0", mockConfig)
 			if err == nil && c.expectedError == nil {
 				//only do these in an error-less context
 				if c.namespace != controller.ConfigmapNamespace {

admiral/pkg/controller/secret/resolver/idpsresolver.go

@@ -39,7 +39,7 @@ func (r IDPSResolver) FetchKubeConfig(secretName string, _ []byte) ([]byte, erro
 	}
 }
 
-func NewIDPSResolver(configFile string) (SecretResolver, error) {
+func NewIDPSResolver(configFile string, clientProvider client.IdpsSdkProvider) (SecretResolver, error) {
 
 	data, err := ioutil.ReadFile(configFile)
 
@@ -53,7 +53,7 @@ func NewIDPSResolver(configFile string) (SecretResolver, error) {
 		return nil, fmt.Errorf("error unmarshaling config file err: %v", err)
 	}
 
-	IdpsClient, err := client.NewIdpsClient(Config)
+	IdpsClient, err := client.NewIdpsClient(Config, clientProvider)
 	if err != nil {
 		glog.Infof("Failed to init IDPS clients in err%v", err)
 		return nil, err

admiral/pkg/controller/secret/resolver/idpsresolver_test.go

@@ -1,12 +1,13 @@
 package resolver
 
 import (
+	"github.com/istio-ecosystem/admiral/admiral/pkg/client"
+	clientMocks "github.com/istio-ecosystem/admiral/admiral/pkg/client/mocks"
+	"github.intuit.com/idps/idps-go-sdk/v3/idps-sdk/items"
 	"io/ioutil"
 	"sync"
 	"testing"
 
-	clientMocks "github.com/istio-ecosystem/admiral/admiral/pkg/client/mocks"
-
 	"gopkg.in/yaml.v2"
 )
 
@@ -62,10 +63,27 @@ func TestFetchKubeConfig(t *testing.T) {
 	}
 }
 
-func TestNewIdpsResolver(t *testing.T) {
+type MockIdpsClientProviderWrapper struct{}
+
+type IdpsClientMock struct{}
+
+func (i IdpsClientMock) GetSecret(variables *items.SecretVariables, version int32) (*items.Secret, error) {
+	secret := items.Secret{
+		SecretName:  "secrets/management/pem",
+		SecretValue: "cmFuZG9tX3NlY3JldA==",
+	}
+	return &secret, nil
+}
+
+func (c *MockIdpsClientProviderWrapper) IdpsClientInstanceFromMap(props map[string]string) (client.IdpsClientInterface, error) {
+	return IdpsClientMock{}, nil
+}
+
+func TestNewIDPSResolver(t *testing.T) {
 	config := "fixtures/admiral-idps-config.yaml"
 
-	idpsResolver,err := NewIDPSResolver(config)
+	mockProvider := &MockIdpsClientProviderWrapper{}
+	idpsResolver, err := NewIDPSResolver(config, mockProvider)
 
 	if err != nil {
 		t.Errorf("Unexpected err %v", err)
@@ -74,4 +92,4 @@ func TestNewIdpsResolver(t *testing.T) {
 	if idpsResolver == nil {
 		t.Errorf("IDPSResolver should never be nil without an error thrown")
 	}
-}
+}

admiral/pkg/controller/secret/secretcontroller.go

@@ -18,6 +18,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/istio-ecosystem/admiral/admiral/pkg/client"
+	idps_sdk "github.intuit.com/idps/idps-go-sdk/v3/idps-sdk"
 	"time"
 
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
@@ -88,6 +90,12 @@ func newClustersStore() *ClusterStore {
 	}
 }
 
+type IdpsSdkWrapper struct{}
+
+func (c *IdpsSdkWrapper) IdpsClientInstanceFromMap(props map[string]string) (client.IdpsClientInterface, error) {
+	return idps_sdk.IdpsClientInstanceFromMap(props)
+}
+
 // NewController returns a new secret controller
 func NewController(
 	kubeclientset kubernetes.Interface,
@@ -121,7 +129,8 @@ func NewController(
 
 	if admiralProfile == common.AdmiralProfileIntuit {
 		log.Info("Initializing Intuit secret resolver")
-		secretResolver, err = resolver.NewIDPSResolver(secretResolverConfig)
+		idpsClientProviderWrapper := &IdpsSdkWrapper{}
+		secretResolver, err = resolver.NewIDPSResolver(secretResolverConfig, idpsClientProviderWrapper)
 	} else if admiralProfile == common.AdmiralProfileDefault {
 		log.Info("Initializing default secret resolver")
 		secretResolver, err = resolver.NewDefaultResolver()