Fixes hashivault connection check

Initially in the #933 it was checking if the vault address is valid or not, but if the url did not have port then it used to return an error and could not proceed further for signing Hence this patch fixes this by checking if the url has port or if the url is a http or https url and then validates if the url is valid so that it can proceed for signing Signed-off-by: PuneetPunamiya <[email protected]>
tektoncd · Oct 13, 2023 · 1eaa3ad · 1eaa3ad
1 parent c29dbc0
commit 1eaa3ad
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 6 deletions.
diff --git a/pkg/chains/signing/kms/kms.go b/pkg/chains/signing/kms/kms.go
@@ -17,6 +17,7 @@ package kms
 import (
 	"context"
 	"crypto"
+	"fmt"
 	"net"
 	"net/url"
 	"time"
@@ -51,11 +52,45 @@ func NewSigner(ctx context.Context, cfg config.KMSSigner) (*Signer, error) {
 			return nil, err
 		}
 
-		conn, err := net.DialTimeout("tcp", vaultAddress.Host, 5*time.Second)
-		if err != nil {
-			return nil, err
+		var vaultUrl *url.URL
+		switch {
+		case vaultAddress.Port() != "":
+			vaultUrl = vaultAddress
+		case vaultAddress.Scheme == "http":
+			vaultUrl = &url.URL{
+				Scheme: vaultAddress.Scheme,
+				Host:   vaultAddress.Host + ":80",
+			}
+		case vaultAddress.Scheme == "https":
+			vaultUrl = &url.URL{
+				Scheme: vaultAddress.Scheme,
+				Host:   vaultAddress.Host + ":443",
+			}
+		case vaultAddress.Scheme == "":
+			vaultUrl = &url.URL{
+				Scheme: "http",
+				Host:   cfg.Auth.Address + ":80",
+			}
+		case vaultAddress.Scheme != "" && vaultAddress.Scheme != "http" && vaultAddress.Scheme != "https":
+			vaultUrl = &url.URL{
+				Scheme: "http",
+				Host:   cfg.Auth.Address,
+			}
+			if vaultUrl.Port() == "" {
+				vaultUrl.Host = cfg.Auth.Address + ":80"
+			}
+		}
+
+		if vaultUrl != nil {
+			conn, err := net.DialTimeout("tcp", vaultUrl.Host, 5*time.Second)
+			if err != nil {
+				fmt.Printf("Error connecting to URL %s: %v\n", vaultUrl, err)
+				return nil, err
+			}
+			defer conn.Close()
+		} else {
+			return nil, fmt.Errorf("Error connecting to URL %s\n", cfg.Auth.Address)
 		}
-		defer conn.Close()
 	}
 
 	// pass through configuration options to RPCAuth used by KMS in sigstore

diff --git a/pkg/chains/signing/kms/kms_test.go b/pkg/chains/signing/kms/kms_test.go
@@ -17,6 +17,9 @@ package kms
 
 import (
 	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/tektoncd/chains/pkg/config"
@@ -26,7 +29,7 @@ func TestInValidVaultAddressTimeout(t *testing.T) {
 	cfg := config.KMSSigner{}
 	cfg.Auth.Address = "http://8.8.8.8:8200"
 
-	_, err := NewSigner(context.TODO(), cfg)
+	_, err := NewSigner(context.Background(), cfg)
 	expectedErrorMessage := "dial tcp 8.8.8.8:8200: i/o timeout"
 	if err.Error() != expectedErrorMessage {
 		t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
@@ -37,9 +40,89 @@ func TestInValidVaultAddressConnectionRefused(t *testing.T) {
 	cfg := config.KMSSigner{}
 	cfg.Auth.Address = "http://127.0.0.1:8200"
 
-	_, err := NewSigner(context.TODO(), cfg)
+	_, err := NewSigner(context.Background(), cfg)
 	expectedErrorMessage := "dial tcp 127.0.0.1:8200: connect: connection refused"
 	if err.Error() != expectedErrorMessage {
 		t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
 	}
 }
+
+func TestValidVaultAddressConnectionWithoutPortAndScheme(t *testing.T) {
+	cfg := config.KMSSigner{}
+	cfg.Auth.Address = "abc.com"
+
+	_, err := NewSigner(context.Background(), cfg)
+	expectedErrorMessage := "no kms provider found for key reference: "
+	if err.Error() != expectedErrorMessage {
+		t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
+	}
+}
+
+func TestValidVaultAddressConnectionWithoutScheme(t *testing.T) {
+	cfg := config.KMSSigner{}
+	cfg.Auth.Address = "abc.com:80"
+
+	_, err := NewSigner(context.Background(), cfg)
+	expectedErrorMessage := "no kms provider found for key reference: "
+	if err.Error() != expectedErrorMessage {
+		t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
+	}
+}
+
+func TestValidVaultAddressConnection(t *testing.T) {
+	t.Run("Validation for Vault Address with HTTP Url", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer server.Close()
+
+		cfg := config.KMSSigner{}
+		cfg.Auth.Address = server.URL
+
+		_, err := NewSigner(context.Background(), cfg)
+		expectedErrorMessage := "no kms provider found for key reference: "
+		if err.Error() != expectedErrorMessage {
+			t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
+		}
+	})
+
+	t.Run("Validation for Vault Address with HTTPS URL", func(t *testing.T) {
+		server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer server.Close()
+
+		cfg := config.KMSSigner{}
+		cfg.Auth.Address = server.URL
+
+		_, err := NewSigner(context.Background(), cfg)
+		expectedErrorMessage := "no kms provider found for key reference: "
+		if err.Error() != expectedErrorMessage {
+			t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
+		}
+	})
+
+	t.Run("Validation for Vault Address with Custom Port URL", func(t *testing.T) {
+		server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer server.Close()
+
+		listener, err := net.Listen("tcp", "127.0.0.1:41227")
+		if err != nil {
+			t.Fatalf("Failed to create listener: %v", err)
+		}
+
+		server.Listener = listener
+		server.Start()
+
+		cfg := config.KMSSigner{}
+		cfg.Auth.Address = "http://127.0.0.1:41227"
+
+		_, err = NewSigner(context.Background(), cfg)
+		expectedErrorMessage := "no kms provider found for key reference: "
+		if err.Error() != expectedErrorMessage {
+			t.Errorf("Expected error message '%s', but got '%s'", expectedErrorMessage, err.Error())
+		}
+	})
+}