Skip to content

Commit

Permalink
Fixes hashivault connection check
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
PuneetPunamiya committed Oct 10, 2023
1 parent 91d4468 commit ec77af2
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 4 deletions.
27 changes: 23 additions & 4 deletions pkg/chains/signing/kms/kms.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package kms
import (
"context"
"crypto"
"fmt"
"net"
"net/url"
"time"
Expand Down Expand Up @@ -51,11 +52,29 @@ 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
if vaultAddress.Port() != "" {
vaultUrl = vaultAddress
} else if vaultAddress.Scheme == "http" {
vaultUrl = &url.URL{
Scheme: vaultAddress.Scheme,
Host: vaultAddress.Host + ":80",
}
} else if vaultAddress.Scheme == "https" {
vaultUrl = &url.URL{
Scheme: vaultAddress.Scheme,
Host: vaultAddress.Host + ":443",
}
}

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()
}
defer conn.Close()
}

// pass through configuration options to RPCAuth used by KMS in sigstore
Expand Down
61 changes: 61 additions & 0 deletions pkg/chains/signing/kms/kms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ package kms

import (
"context"
"net"
"net/http"
"net/http/httptest"
"testing"

"github.com/tektoncd/chains/pkg/config"
Expand All @@ -43,3 +46,61 @@ func TestInValidVaultAddressConnectionRefused(t *testing.T) {
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.TODO(), 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.TODO(), 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.TODO(), 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())
}
})
}

0 comments on commit ec77af2

Please sign in to comment.