Skip to content

Commit

Permalink
feature/tracing-credentials: 'secure' --> 'tls' and 'insecure' --> 'u…
Browse files Browse the repository at this point in the history
…nencrypted'
  • Loading branch information
patrickhuie19 committed Nov 22, 2023
1 parent 06dcc79 commit de6ad7a
Show file tree
Hide file tree
Showing 26 changed files with 237 additions and 90 deletions.
2 changes: 1 addition & 1 deletion GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,4 @@ help:
@echo " \/ \/ \/ \/ \/ \/"
@echo ""
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | \
awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
4 changes: 0 additions & 4 deletions core/cmd/cosmos_node_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ func TestShell_IndexCosmosNodes(t *testing.T) {
chainID := cosmostest.RandomChainID()
node := coscfg.Node{
Name: ptr("second"),
<<<<<<< HEAD
TendermintURL: config.MustParseURL("http://tender.mint.test/bombay-12"),
=======
TendermintURL: relayconfig.MustParseURL("http://tender.mint.test/bombay-12"),
>>>>>>> 34eecf3b1d (feature/tracing-credentials: bumping relay and lint)
}
chain := coscfg.TOMLConfig{
ChainID: ptr(chainID),
Expand Down
8 changes: 0 additions & 8 deletions core/cmd/solana_node_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,11 @@ func TestShell_IndexSolanaNodes(t *testing.T) {
id := solanatest.RandomChainID()
node1 := solcfg.Node{
Name: ptr("first"),
<<<<<<< HEAD
URL: config.MustParseURL("https://solana1.example"),
}
node2 := solcfg.Node{
Name: ptr("second"),
URL: config.MustParseURL("https://solana2.example"),
=======
URL: relayconfig.MustParseURL("https://solana1.example"),
}
node2 := solcfg.Node{
Name: ptr("second"),
URL: relayconfig.MustParseURL("https://solana2.example"),
>>>>>>> 34eecf3b1d (feature/tracing-credentials: bumping relay and lint)
}
chain := solana.TOMLConfig{
ChainID: &id,
Expand Down
4 changes: 0 additions & 4 deletions core/cmd/starknet_node_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

<<<<<<< HEAD
commoncfg "github.com/smartcontractkit/chainlink-common/pkg/config"
=======
relayconfig "github.com/smartcontractkit/chainlink-relay/pkg/config"
>>>>>>> 34eecf3b1d (feature/tracing-credentials: bumping relay and lint)
"github.com/smartcontractkit/chainlink-starknet/relayer/pkg/chainlink/config"

"github.com/smartcontractkit/chainlink/v2/core/cmd"
Expand Down
4 changes: 2 additions & 2 deletions core/config/docs/core.toml
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,8 @@ CollectorTarget = 'localhost:4317' # Example
NodeID = 'NodeID' # Example
# SamplingRatio is the ratio of traces to sample for this node.
SamplingRatio = 1.0 # Example
# Mode is a string value. 'secure' or 'insecure' are the only values allowed. If set to 'insecure', TLSCertPath can be unset, meaning traces will be sent over plaintext to the collector.
Mode = 'secure' # Default
# Mode is a string value. 'tls' or 'unencrypted' are the only values allowed. If set to 'unencrypted', TLSCertPath can be unset, meaning traces will be sent over plaintext to the collector.
Mode = 'tls' # Default
# TLSCertPath is the file path to the TLS certificate used for secure communication with an OTEL Tracing Collector.
TLSCertPath = '/path/to/cert.pem' # Example

Expand Down
47 changes: 30 additions & 17 deletions core/config/toml/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1495,30 +1495,39 @@ func (t *Tracing) ValidateConfig() (err error) {
}
}

if t.CollectorTarget != nil {
ok := isValidURI(*t.CollectorTarget)
if !ok {
err = multierr.Append(err, configutils.ErrInvalid{Name: "CollectorTarget", Value: *t.CollectorTarget, Msg: "must be a valid URI"})
}
}

if t.Mode != nil {
switch *t.Mode {
case "secure":
case "tls":
// TLSCertPath must be set
if t.TLSCertPath == nil {
err = multierr.Append(err, configutils.ErrMissing{Name: "TLSCertPath", Msg: "must be set when Tracing.Mode is secure"})
err = multierr.Append(err, configutils.ErrMissing{Name: "TLSCertPath", Msg: "must be set when Tracing.Mode is tls"})
} else {
ok := isValidFilePath(*t.TLSCertPath)
if !ok {
err = multierr.Append(err, configutils.ErrInvalid{Name: "TLSCertPath", Value: *t.TLSCertPath, Msg: "must be a valid file path"})
}
}
case "insecure":
case "unencrypted":
// no-op
default:
// Mode must be either "secure" or "insecure"
err = multierr.Append(err, configutils.ErrInvalid{Name: "Mode", Value: *t.Mode, Msg: "must be either 'secure' or 'insecure'"})
// Mode must be either "tls" or "unencrypted"
err = multierr.Append(err, configutils.ErrInvalid{Name: "Mode", Value: *t.Mode, Msg: "must be either 'tls' or 'unencrypted'"})
}
}

if t.CollectorTarget != nil && t.Mode != nil {
switch *t.Mode {
case "tls":
if !isValidURI(*t.CollectorTarget) {
err = multierr.Append(err, configutils.ErrInvalid{Name: "CollectorTarget", Value: *t.CollectorTarget, Msg: "must be a valid URI"})
}
case "unencrypted":
// Unencrypted traces can not be sent to external networks
if !isValidLocalURI(*t.CollectorTarget) {
err = multierr.Append(err, configutils.ErrInvalid{Name: "CollectorTarget", Value: *t.CollectorTarget, Msg: "must be a valid local URI"})
}
default:
// no-op
}
}

Expand All @@ -1527,15 +1536,19 @@ func (t *Tracing) ValidateConfig() (err error) {

var hostnameRegex = regexp.MustCompile(`^[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)*$`)

// Validates uri is valid external or local URI
func isValidURI(uri string) bool {
if strings.Contains(uri, "://") {
// Standard URI check
_, _ = url.ParseRequestURI(uri)
// TODO: BCF-2703. Handle error. All external addresses currently fail validation until we have secure transport to external networks.
return false
_, err := url.ParseRequestURI(uri)
return err == nil
}

// For URIs like "otel-collector:4317"
return isValidLocalURI(uri)
}

// Validates uri is a valid local URI
// External URIs (e.g. http://) are not valid local URIs, and will return false.
func isValidLocalURI(uri string) bool {
parts := strings.Split(uri, ":")
if len(parts) == 2 {
host, port := parts[0], parts[1]
Expand Down
104 changes: 82 additions & 22 deletions core/config/toml/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,55 +185,115 @@ func TestTracing_ValidateCollectorTarget(t *testing.T) {
tests := []struct {
name string
collectorTarget *string
mode *string
wantErr bool
errMsg string
}{
{
name: "valid http address",
name: "valid http address in tls mode",
collectorTarget: ptr("https://testing.collector.dev"),
mode: ptr("tls"),
wantErr: false,
},
{
name: "valid http address in unencrypted mode",
collectorTarget: ptr("https://localhost:4317"),
// TODO: BCF-2703. Re-enable when we have secure transport to otel collectors in external networks
wantErr: true,
errMsg: "CollectorTarget: invalid value (https://localhost:4317): must be a valid URI",
mode: ptr("unencrypted"),
wantErr: true,
errMsg: "CollectorTarget: invalid value (https://localhost:4317): must be a valid local URI",
},
// Tracing.Mode = 'tls'
{
name: "valid localhost address",
collectorTarget: ptr("localhost:4317"),
mode: ptr("tls"),
wantErr: false,
},
{
name: "valid docker address",
collectorTarget: ptr("otel-collector:4317"),
mode: ptr("tls"),
wantErr: false,
},
{
name: "valid IP address",
collectorTarget: ptr("192.168.1.1:4317"),
mode: ptr("tls"),
wantErr: false,
},
{
name: "invalid port",
collectorTarget: ptr("localhost:invalid"),
wantErr: true,
mode: ptr("tls"),
errMsg: "CollectorTarget: invalid value (localhost:invalid): must be a valid URI",
},
{
name: "invalid address",
collectorTarget: ptr("invalid address"),
wantErr: true,
mode: ptr("tls"),
errMsg: "CollectorTarget: invalid value (invalid address): must be a valid URI",
},
{
name: "nil CollectorTarget",
collectorTarget: ptr(""),
wantErr: true,
mode: ptr("tls"),
errMsg: "CollectorTarget: invalid value (): must be a valid URI",
},
// Tracing.Mode = 'unencrypted'
{
name: "valid localhost address",
collectorTarget: ptr("localhost:4317"),
mode: ptr("unencrypted"),
wantErr: false,
},
{
name: "valid docker address",
collectorTarget: ptr("otel-collector:4317"),
mode: ptr("unencrypted"),
wantErr: false,
},
{
name: "valid IP address",
collectorTarget: ptr("192.168.1.1:4317"),
mode: ptr("unencrypted"),
wantErr: false,
},
{
name: "invalid port",
collectorTarget: ptr("localhost:invalid"),
wantErr: true,
mode: ptr("unencrypted"),
errMsg: "CollectorTarget: invalid value (localhost:invalid): must be a valid local URI",
},
{
name: "invalid address",
collectorTarget: ptr("invalid address"),
wantErr: true,
mode: ptr("unencrypted"),
errMsg: "CollectorTarget: invalid value (invalid address): must be a valid local URI",
},
{
name: "nil CollectorTarget",
collectorTarget: ptr(""),
wantErr: true,
mode: ptr("unencrypted"),
errMsg: "CollectorTarget: invalid value (): must be a valid local URI",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var tlsCertPath string
if *tt.mode == "tls" {
tlsCertPath = "/path/to/cert.pem"
}
tracing := &Tracing{
Enabled: ptr(true),
TLSCertPath: &tlsCertPath,
Mode: tt.mode,
CollectorTarget: tt.collectorTarget,
}

Expand Down Expand Up @@ -310,8 +370,8 @@ func TestTracing_ValidateSamplingRatio(t *testing.T) {
}

func TestTracing_ValidateTLSCertPath(t *testing.T) {
// tests for Tracing.Mode = 'secure'
secure_tests := []struct {
// tests for Tracing.Mode = 'tls'
tls_tests := []struct {
name string
tlsCertPath *string
wantErr bool
Expand Down Expand Up @@ -341,8 +401,8 @@ func TestTracing_ValidateTLSCertPath(t *testing.T) {
},
}

// tests for Tracing.Mode = 'insecure'
insecure_tests := []struct {
// tests for Tracing.Mode = 'unencrypted'
unencrypted_tests := []struct {
name string
tlsCertPath *string
wantErr bool
Expand Down Expand Up @@ -370,10 +430,10 @@ func TestTracing_ValidateTLSCertPath(t *testing.T) {
},
}

for _, tt := range secure_tests {
for _, tt := range tls_tests {
t.Run(tt.name, func(t *testing.T) {
tracing := &Tracing{
Mode: ptr("secure"),
Mode: ptr("tls"),
TLSCertPath: tt.tlsCertPath,
Enabled: ptr(true),
}
Expand All @@ -389,10 +449,10 @@ func TestTracing_ValidateTLSCertPath(t *testing.T) {
})
}

for _, tt := range insecure_tests {
for _, tt := range unencrypted_tests {
t.Run(tt.name, func(t *testing.T) {
tracing := &Tracing{
Mode: ptr("insecure"),
Mode: ptr("unencrypted"),
TLSCertPath: tt.tlsCertPath,
Enabled: ptr(true),
}
Expand All @@ -418,27 +478,27 @@ func TestTracing_ValidateMode(t *testing.T) {
errMsg string
}{
{
name: "secure mode with valid TLS path",
mode: ptr("secure"),
name: "tls mode with valid TLS path",
mode: ptr("tls"),
tlsCertPath: ptr("/path/to/cert.pem"),
wantErr: false,
},
{
name: "secure mode without TLS path",
mode: ptr("secure"),
name: "tls mode without TLS path",
mode: ptr("tls"),
tlsCertPath: nil,
wantErr: true,
errMsg: "TLSCertPath: missing: must be set when Tracing.Mode is secure",
errMsg: "TLSCertPath: missing: must be set when Tracing.Mode is tls",
},
{
name: "insecure mode with TLS path",
mode: ptr("insecure"),
name: "unencrypted mode with TLS path",
mode: ptr("unencrypted"),
tlsCertPath: ptr("/path/to/cert.pem"),
wantErr: false,
},
{
name: "insecure mode without TLS path",
mode: ptr("insecure"),
name: "unencrypted mode without TLS path",
mode: ptr("unencrypted"),
tlsCertPath: nil,
wantErr: false,
},
Expand All @@ -447,7 +507,7 @@ func TestTracing_ValidateMode(t *testing.T) {
mode: ptr("unknown"),
tlsCertPath: nil,
wantErr: true,
errMsg: "Mode: invalid value (unknown): must be either 'secure' or 'insecure'",
errMsg: "Mode: invalid value (unknown): must be either 'tls' or 'unencrypted'",
},
}

Expand Down
13 changes: 7 additions & 6 deletions core/services/chainlink/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,19 @@ func (c *Config) TOMLString() (string, error) {

// warnings aggregates warnings from valueWarnings and deprecationWarnings
func (c *Config) warnings() (err error) {
_, warning := utils.MultiErrorList(c.deprecationWarnings())
err = c.valueWarnings()
err = multierr.Append(warning, err)
return
deprecationErr := c.deprecationWarnings()
warningErr := c.valueWarnings()
err = multierr.Append(deprecationErr, warningErr)
_, list := utils.MultiErrorList(err)
return list
}

// valueWarnings returns an error if the Config contains values that hint at misconfiguration before defaults are applied.
func (c *Config) valueWarnings() (err error) {
if c.Tracing.Enabled != nil && *c.Tracing.Enabled {
if c.Tracing.Mode != nil && *c.Tracing.Mode == "insecure" {
if c.Tracing.Mode != nil && *c.Tracing.Mode == "unencrypted" {
if c.Tracing.TLSCertPath != nil {
err = multierr.Append(err, config.ErrInvalid{Name: "Tracing.TLSCertPath", Value: *c.Tracing.TLSCertPath, Msg: "must be empty when Tracing.Mode is 'insecure'"})
err = multierr.Append(err, config.ErrInvalid{Name: "Tracing.TLSCertPath", Value: *c.Tracing.TLSCertPath, Msg: "must be empty when Tracing.Mode is 'unencrypted'"})
}
}
}
Expand Down
Loading

0 comments on commit de6ad7a

Please sign in to comment.