Merge pull request #458 from jetstack/venafi-agent-configuration

Refactor the venafi-cloud configuration

Author: tfadeyi

cmd/agent.go

@@ -41,7 +41,7 @@ var agentRBACCmd = &cobra.Command{
 		if err != nil {
 			log.Fatalf("Failed to read config file: %s", err)
 		}
-		config, err := agent.ParseConfig(b)
+		config, err := agent.ParseConfig(b, false)
 		if err != nil {
 			log.Fatalf("Failed to parse config file: %s", err)
 		}

pkg/agent/config.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-multierror"
+	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/datagatherer"
 	"github.com/jetstack/preflight/pkg/datagatherer/k8s"
 	"github.com/jetstack/preflight/pkg/datagatherer/local"
@@ -126,22 +127,19 @@ func (c *Config) Dump() (string, error) {
 	return string(d), nil
 }
 
-func (c *Config) validate() error {
+func (c *Config) validate(isVenafiCloudMode bool) error {
 	var result *multierror.Error
 
 	// configured for Venafi Cloud
 	if c.VenafiCloud != nil {
-		if c.VenafiCloud.UploaderID == "" {
-			result = multierror.Append(result, fmt.Errorf("upload_id is required in Venafi Cloud mode"))
-		}
 		if c.VenafiCloud.UploadPath == "" {
 			result = multierror.Append(result, fmt.Errorf("upload_path is required in Venafi Cloud mode"))
 		}
 
 		if _, err := url.Parse(c.VenafiCloud.UploadPath); err != nil {
 			result = multierror.Append(result, fmt.Errorf("upload_path is not a valid URL"))
 		}
-	} else {
+	} else if !isVenafiCloudMode {
 		if c.OrganizationID == "" {
 			result = multierror.Append(result, fmt.Errorf("organization_id is required"))
 		}
@@ -169,7 +167,7 @@ func (c *Config) validate() error {
 }
 
 // ParseConfig reads config into a struct used to configure running agents
-func ParseConfig(data []byte) (Config, error) {
+func ParseConfig(data []byte, isVenafiCloudMode bool) (Config, error) {
 	var config Config
 
 	err := yaml.Unmarshal(data, &config)
@@ -178,18 +176,18 @@ func ParseConfig(data []byte) (Config, error) {
 	}
 
 	if config.Server == "" && config.Endpoint.Host == "" && config.Endpoint.Path == "" {
+		config.Server = "https://preflight.jetstack.io"
 		if config.VenafiCloud != nil {
-			config.Server = "https://api.venafi.cloud"
-		} else {
-			config.Server = "https://preflight.jetstack.io"
+			config.Server = client.VenafiCloudProdURL
 		}
 	}
 
 	if config.Endpoint.Protocol == "" && config.Server == "" {
 		config.Endpoint.Protocol = "http"
 	}
 
-	if err = config.validate(); err != nil {
+	err = config.validate(isVenafiCloudMode)
+	if err != nil {
 		return config, err
 	}
 

pkg/agent/config_test.go

@@ -25,7 +25,7 @@ func TestValidConfigLoad(t *testing.T) {
       output-path: "/nothome"
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -68,7 +68,7 @@ func TestValidConfigWithEndpointLoad(t *testing.T) {
           always-fail: false
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -114,7 +114,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) {
         upload_path: "/testing/path"
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -149,7 +149,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) {
 func TestInvalidConfigError(t *testing.T) {
 	configFileContents := `data-gatherers: "things"`
 
-	_, parseError := ParseConfig([]byte(configFileContents))
+	_, parseError := ParseConfig([]byte(configFileContents), false)
 
 	expectedError := fmt.Errorf("yaml: unmarshal errors:\n  line 1: cannot unmarshal !!str `things` into []agent.DataGatherer")
 
@@ -159,26 +159,35 @@ func TestInvalidConfigError(t *testing.T) {
 }
 
 func TestMissingConfigError(t *testing.T) {
-	_, parseError := ParseConfig([]byte(""))
-
-	if parseError == nil {
-		t.Fatalf("expected error, got nil")
-	}
-
-	expectedErrorLines := []string{
-		"2 errors occurred:",
-		"\t* organization_id is required",
-		"\t* cluster_id is required",
-		"\n",
-	}
-
-	expectedError := strings.Join(expectedErrorLines, "\n")
-
-	gotError := parseError.Error()
-
-	if gotError != expectedError {
-		t.Errorf("\ngot=\n%v\nwant=\n%s\ndiff=\n%s", gotError, expectedError, diff.Diff(gotError, expectedError))
-	}
+	t.Run("fail to parse config if organization_id or cluster_id are missing (venafi-cloud not enabled)", func(t *testing.T) {
+		_, parseError := ParseConfig([]byte(""), false)
+
+		if parseError == nil {
+			t.Fatalf("expected error, got nil")
+		}
+
+		expectedErrorLines := []string{
+			"2 errors occurred:",
+			"\t* organization_id is required",
+			"\t* cluster_id is required",
+			"\n",
+		}
+
+		expectedError := strings.Join(expectedErrorLines, "\n")
+
+		gotError := parseError.Error()
+
+		if gotError != expectedError {
+			t.Errorf("\ngot=\n%v\nwant=\n%s\ndiff=\n%s", gotError, expectedError, diff.Diff(gotError, expectedError))
+		}
+	})
+	t.Run("successfully parse config if organization_id or cluster_id are missing (venafi-cloud is enabled)", func(t *testing.T) {
+		_, parseError := ParseConfig([]byte(""), true)
+
+		if parseError != nil {
+			t.Fatalf("unxexpected error, no error should have occured when parsing configuration: %s", parseError)
+		}
+	})
 }
 
 func TestPartialMissingConfigError(t *testing.T) {
@@ -190,7 +199,7 @@ func TestPartialMissingConfigError(t *testing.T) {
       organization_id: "example"
       cluster_id: "example-cluster"
       data-gatherers:
-        - kind: dummy`))
+        - kind: dummy`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")
@@ -218,7 +227,7 @@ func TestInvalidServerError(t *testing.T) {
       cluster_id: "my_cluster"
       data-gatherers:
         - kind: dummy
-          name: dummy`))
+          name: dummy`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")
@@ -246,7 +255,7 @@ func TestInvalidDataGathered(t *testing.T) {
         path: /api/v1/data
       schedule: "* * * * *"
       data-gatherers:
-        - kind: "foo"`))
+        - kind: "foo"`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")

pkg/agent/run.go

@@ -198,11 +198,16 @@ func getConfiguration() (Config, client.Client) {
 		log.Fatalf("Failed to read config file: %s", err)
 	}
 
-	config, err := ParseConfig(b)
+	config, err := ParseConfig(b, VenafiCloudMode)
 	if err != nil {
 		log.Fatalf("Failed to parse config file: %s", err)
 	}
 
+	if VenafiCloudMode {
+		// if the venafi-cloud mode is enabled override config.Server
+		config.Server = client.VenafiCloudProdURL
+	}
+
 	baseURL := config.Server
 	if baseURL == "" {
 		log.Printf("Using deprecated Endpoint configuration. User Server instead.")
@@ -274,11 +279,15 @@ func createCredentialClient(credentials client.Credentials, config Config, agent
 	switch creds := credentials.(type) {
 	case *client.VenafiSvcAccountCredentials:
 		log.Println("Venafi Cloud mode was specified, using Venafi Service Account authentication.")
-		// check if config has Venafi Cloud data
-		if config.VenafiCloud == nil {
-			log.Fatalf("Failed to find config for venafi-cloud: required for Venafi Cloud mode")
+		// check if config has Venafi Cloud data, use config data if it's present
+		uploaderID := creds.ClientID
+		uploadPath := ""
+		if config.VenafiCloud != nil {
+			log.Println("Loading uploader_id and upload_path from \"venafi-cloud\" configuration.")
+			uploaderID = config.VenafiCloud.UploaderID
+			uploadPath = config.VenafiCloud.UploadPath
 		}
-		return client.NewVenafiCloudClient(agentMetadata, creds, baseURL, config.VenafiCloud.UploaderID, config.VenafiCloud.UploadPath)
+		return client.NewVenafiCloudClient(agentMetadata, creds, baseURL, uploaderID, uploadPath)
 
 	case *client.OAuthCredentials:
 		log.Println("A credentials file was specified, using oauth authentication.")

pkg/client/client_venafi_cloud.go

@@ -70,9 +70,11 @@ type (
 )
 
 const (
-	vaasProdURL         = "https://api.venafi.cloud"
-	accessTokenEndpoint = "/v1/oauth/token/serviceaccount"
-	requiredGrantType   = "urn:ietf:params:oauth:grant-type:jwt-bearer"
+	// URL for the venafi-cloud backend services
+	VenafiCloudProdURL               = "https://api.venafi.cloud"
+	defaultVenafiCloudUploadEndpoint = "v1/tlspk/uploads"
+	accessTokenEndpoint              = "/v1/oauth/token/serviceaccount"
+	requiredGrantType                = "urn:ietf:params:oauth:grant-type:jwt-bearer"
 )
 
 // NewVenafiCloudClient returns a new instance of the VenafiCloudClient type that will perform HTTP requests using a bearer token
@@ -93,6 +95,11 @@ func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiS
 		return nil, fmt.Errorf("cannot create VenafiCloudClient: invalid Venafi Cloud client configuration")
 	}
 
+	if uploadPath == "" {
+		// if the uploadPath is not given, use default upload path
+		uploadPath = defaultVenafiCloudUploadEndpoint
+	}
+
 	return &VenafiCloudClient{
 		agentMetadata: agentMetadata,
 		credentials:   credentials,
@@ -283,7 +290,7 @@ func (c *VenafiCloudClient) sendHTTPRequest(request *http.Request, responseObjec
 }
 
 func (c *VenafiCloudClient) generateAndSignJwtToken() (string, error) {
-	prodURL, err := url.Parse(vaasProdURL)
+	prodURL, err := url.Parse(VenafiCloudProdURL)
 	if err != nil {
 		return "", err
 	}