Skip to content

Commit

Permalink
rpk: do not modify redpanda.rpc_server_tls field
Browse files Browse the repository at this point in the history
rpk now will treat this field like an unmanaged
field, which means that we can have either a list,
or an element, and rpk will respect that.
  • Loading branch information
r-vasquez authored and twmb committed Mar 10, 2023
1 parent 7fc2351 commit 58795aa
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
1 change: 0 additions & 1 deletion src/go/rpk/pkg/cli/cmd/debug/bundle_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,6 @@ func saveConfig(ps *stepParams, conf *config.Config) step {
// We want to redact any blindly decoded parameters.
redactOtherMap(conf.Other)
redactOtherMap(conf.Redpanda.Other)
redactServerTLSSlice(conf.Redpanda.RPCServerTLS)
redactServerTLSSlice(conf.Redpanda.KafkaAPITLS)
redactServerTLSSlice(conf.Redpanda.AdminAPITLS)
if conf.SchemaRegistry != nil {
Expand Down
1 change: 0 additions & 1 deletion src/go/rpk/pkg/config/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ type RedpandaNodeConfig struct {
Rack string `yaml:"rack,omitempty" json:"rack"`
SeedServers []SeedServer `yaml:"seed_servers" json:"seed_servers"`
RPCServer SocketAddress `yaml:"rpc_server,omitempty" json:"rpc_server"`
RPCServerTLS []ServerTLS `yaml:"rpc_server_tls,omitempty" json:"rpc_server_tls"`
KafkaAPI []NamedAuthNSocketAddress `yaml:"kafka_api,omitempty" json:"kafka_api"`
KafkaAPITLS []ServerTLS `yaml:"kafka_api_tls,omitempty" json:"kafka_api_tls"`
AdminAPI []NamedSocketAddress `yaml:"admin,omitempty" json:"admin"`
Expand Down
16 changes: 14 additions & 2 deletions src/go/rpk/pkg/config/weak.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ package config
import (
"errors"
"fmt"
"os"
"reflect"
"strconv"
"sync"

"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -325,14 +327,16 @@ func (c *Config) UnmarshalYAML(n *yaml.Node) error {
return nil
}

// once is used to ensure that we only print the rpc_server_tls bug warning once.
var once sync.Once

func (rpc *RedpandaNodeConfig) UnmarshalYAML(n *yaml.Node) error {
var internal struct {
Directory weakString `yaml:"data_directory"`
ID weakInt `yaml:"node_id" `
Rack weakString `yaml:"rack"`
SeedServers seedServers `yaml:"seed_servers"`
RPCServer SocketAddress `yaml:"rpc_server"`
RPCServerTLS serverTLSArray `yaml:"rpc_server_tls"`
KafkaAPI namedAuthNSocketAddresses `yaml:"kafka_api"`
KafkaAPITLS serverTLSArray `yaml:"kafka_api_tls"`
AdminAPI namedSocketAddresses `yaml:"admin"`
Expand All @@ -350,12 +354,20 @@ func (rpc *RedpandaNodeConfig) UnmarshalYAML(n *yaml.Node) error {
if err := n.Decode(&internal); err != nil {
return err
}

// redpanda won't recognize rpc_server_tls if is a list.
v := reflect.ValueOf(internal.Other["rpc_server_tls"])
if v.Kind() == reflect.Slice {
once.Do(func() {
fmt.Fprintf(os.Stderr, "WARNING: Due to an old rpk bug, your redpanda.yaml's redpanda.rpc_server_tls property is an array, and redpanda reads the field as a struct. rpk cannot automatically fix this: brokers would not be able to rejoin the cluster during a rolling upgrade. To enable TLS on broker RPC ports, you must turn off your cluster, switch the redpanda.rpc_server_tls field to a struct, and then turn your cluster back on. To switch from a list to a struct, replace the single dash under redpanda.rpc_server_tls with a space. This message will continue to appear while redpanda.rpc_server_tls exists and is an array\n")
})
}

rpc.Directory = string(internal.Directory)
rpc.ID = int(internal.ID)
rpc.Rack = string(internal.Rack)
rpc.SeedServers = internal.SeedServers
rpc.RPCServer = internal.RPCServer
rpc.RPCServerTLS = internal.RPCServerTLS
rpc.KafkaAPI = internal.KafkaAPI
rpc.KafkaAPITLS = internal.KafkaAPITLS
rpc.AdminAPI = internal.AdminAPI
Expand Down
21 changes: 13 additions & 8 deletions src/go/rpk/pkg/config/weak_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -980,10 +980,7 @@ rpk:
AdminAPITLS: []ServerTLS{
{Enabled: false, CertFile: "certs/tls-cert.pem"},
},
RPCServer: SocketAddress{"0.0.0.0", 33145},
RPCServerTLS: []ServerTLS{
{RequireClientAuth: false, TruststoreFile: "certs/tls-ca.pem"},
},
RPCServer: SocketAddress{"0.0.0.0", 33145},
AdvertisedRPCAPI: &SocketAddress{"0.0.0.0", 33145},
KafkaAPI: []NamedAuthNSocketAddress{
{"0.0.0.0", 9092, "internal", nil},
Expand All @@ -1002,6 +999,13 @@ rpk:
},
Other: map[string]interface{}{
"enable_admin_api": true,
// This one is a slice
"rpc_server_tls": []interface{}{
map[string]interface{}{
"require_client_auth": false,
"truststore_file": "certs/tls-ca.pem",
},
},
},
},
Pandaproxy: &Pandaproxy{
Expand Down Expand Up @@ -1212,10 +1216,7 @@ rpk:
AdminAPITLS: []ServerTLS{
{Enabled: false, CertFile: "certs/tls-cert.pem"},
},
RPCServer: SocketAddress{"0.0.0.0", 33145},
RPCServerTLS: []ServerTLS{
{RequireClientAuth: false, TruststoreFile: "certs/tls-ca.pem"},
},
RPCServer: SocketAddress{"0.0.0.0", 33145},
AdvertisedRPCAPI: &SocketAddress{"0.0.0.0", 33145},
KafkaAPI: []NamedAuthNSocketAddress{
{"0.0.0.0", 9092, "internal", nil},
Expand All @@ -1236,6 +1237,10 @@ rpk:
},
Other: map[string]interface{}{
"enable_admin_api": true,
"rpc_server_tls": map[string]interface{}{
"require_client_auth": false,
"truststore_file": "certs/tls-ca.pem",
},
},
},
Pandaproxy: &Pandaproxy{
Expand Down

0 comments on commit 58795aa

Please sign in to comment.