Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update codis fe / proxy with tls; topom updated #2383

Open
wants to merge 27 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9658b06
update codis proxy to accept tls connection
sprappcom Feb 1, 2024
6ec04e3
tls for proxy works, tls for fe in progress
sprappcom Feb 1, 2024
10b22fc
codis fe tls done
sprappcom Feb 1, 2024
083b737
codis tls fe minor update
sprappcom Feb 1, 2024
4517f87
codis fe minor update
sprappcom Feb 1, 2024
342d893
@chenmin1992 remove my own topom
sprappcom Feb 1, 2024
8ad0b61
optimize code
sprappcom Feb 1, 2024
e233d10
Delete codis/pkg/proxy/.proxy.go.swo
kolinfluence Mar 14, 2024
e99e587
Delete codis/admin/.codis-fe-admin.sh.swo
kolinfluence Mar 14, 2024
c631795
Update store.go
kolinfluence Mar 14, 2024
7b765b7
Update config.go
kolinfluence Mar 14, 2024
6eb8e3a
Update Makefile
kolinfluence Mar 14, 2024
f2cfcee
Update config.go
kolinfluence Mar 14, 2024
b8bda63
Update proxy.toml
kolinfluence Mar 14, 2024
6a8a404
Update proxy.toml
kolinfluence Mar 14, 2024
10038c3
Update codis-fe-admin.sh
kolinfluence Mar 14, 2024
7f78d00
Update dashboard.toml
kolinfluence Mar 14, 2024
8d00a6f
Update topom.go
kolinfluence Mar 14, 2024
e8f24eb
Update main.go
kolinfluence Jul 5, 2024
86afbe7
Update store.go
kolinfluence Jul 5, 2024
1a4e5c4
Update proxy.go
kolinfluence Jul 5, 2024
b9cf0ae
Update topom.go
kolinfluence Jul 5, 2024
a592cc6
Rename cert.pem to cert.pem-dummy
kolinfluence Jul 5, 2024
b7c8ba1
Rename key.pem to key.pem-dummy
kolinfluence Jul 5, 2024
5cfa142
Delete codis/key.pem-dummy
kolinfluence Jul 5, 2024
0d607fa
Delete codis/cert.pem-dummy
kolinfluence Jul 5, 2024
c491c6a
Merge branch 'unstable' into unstable
kolinfluence Jul 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion codis/admin/codis-fe-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ CODIS_FE_DAEMON_FILE=$CODIS_LOG_DIR/codis-fe.out
COORDINATOR_NAME="filesystem"
COORDINATOR_ADDR="/tmp/codis"
CODIS_FE_ADDR="0.0.0.0:9090"
CODIS_FE_TLS=1
CODIS_FE_TLS_KEY="/usr/local/src/pika/codis/key.pem"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment this code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help add the rest?

CODIS_FE_TLS_CERT="/usr/local/src/pika/codis/cert.pem"

echo $CODIS_FE_CONF_FILE

Expand All @@ -36,7 +39,7 @@ start)
fi
fi
nohup "$CODIS_FE_BIN" "--assets-dir=${CODIS_FE_ASSETS_DIR}" "--$COORDINATOR_NAME=$COORDINATOR_ADDR" \
"--log=$CODIS_FE_LOG_FILE" "--pidfile=$CODIS_FE_PID_FILE" "--log-level=INFO" "--listen=$CODIS_FE_ADDR" > "$CODIS_FE_DAEMON_FILE" 2>&1 < /dev/null &
"--tls=$CODIS_FE_TLS" "--tls-key=$CODIS_FE_TLS_KEY" "--tls-cert=$CODIS_FE_TLS_CERT" "--log=$CODIS_FE_LOG_FILE" "--pidfile=$CODIS_FE_PID_FILE" "--log-level=INFO" "--listen=$CODIS_FE_ADDR" > "$CODIS_FE_DAEMON_FILE" 2>&1 < /dev/null &
;;
start-foreground)
$CODIS_FE_BIN "--assets-dir=${CODIS_FE_ASSETS_DIR}" "--$COORDINATOR_NAME=$COORDINATOR_ADDR" \
Expand Down
41 changes: 37 additions & 4 deletions codis/cmd/fe/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package main

import (
"crypto/tls"
"encoding/json"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -56,7 +57,7 @@ func init() {
func main() {
const usage = `
Usage:
codis-fe [--ncpu=N] [--log=FILE] [--log-level=LEVEL] [--assets-dir=PATH] [--pidfile=FILE] (--dashboard-list=FILE|--zookeeper=ADDR [--zookeeper-auth=USR:PWD]|--etcd=ADDR [--etcd-auth=USR:PWD]|--filesystem=ROOT) --listen=ADDR
codis-fe [--ncpu=N] [--log=FILE] [--log-level=LEVEL] [--assets-dir=PATH] [--pidfile=FILE] (--dashboard-list=FILE|--zookeeper=ADDR [--zookeeper-auth=USR:PWD]|--etcd=ADDR [--etcd-auth=USR:PWD]|--filesystem=ROOT) --listen=ADDR [--tls-cert=FILE] [--tls-key=FILE] [--tls=N]
codis-fe --version

Options:
Expand Down Expand Up @@ -196,10 +197,42 @@ Options:
m.MapTo(r, (*martini.Routes)(nil))
m.Action(r.Handle)

l, err := net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
var l net.Listener

if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {

// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}

// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}

// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}


}else{

l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}

Comment on lines +202 to +232
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve error handling and logging for TLS setup.

Ensure that all potential errors are logged appropriately and provide more context.

	if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
		// Load server certificate and key
		certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
		if certFile == "" || keyFile == "" {
			log.Panic("TLS certificate or key file path must be provided")
		}
		cert, err := tls.LoadX509KeyPair(certFile, keyFile)
		if err != nil {
			log.PanicErrorf(err, "failed to load key pair: %s", err)
		}

		// Create a TLS config
		tlsConfig := &tls.Config{
			Certificates: []tls.Certificate{cert},
		}

		// Create a TLS listener
		l, err = tls.Listen("tcp", listen, tlsConfig)
		if err != nil {
			log.PanicErrorf(err, "TLS listen on %s failed: %v", listen, err)
		}
		log.Warnf("TLS enabled, listening on %s", listen)
	} else {
		l, err = net.Listen("tcp", listen)
		if err != nil {
			log.PanicErrorf(err, "listen on %s failed: %v", listen, err)
		}
		log.Warnf("Listening on %s without TLS", listen)
	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}
// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}
// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}
}else{
l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}
if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}
// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}
// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "TLS listen on %s failed: %v", listen, err)
}
log.Warnf("TLS enabled, listening on %s", listen)
} else {
l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen on %s failed: %v", listen, err)
}
log.Warnf("Listening on %s without TLS", listen)
}

}


defer l.Close()

if s, ok := utils.Argument(d, "--pidfile"); ok {
Expand Down
3 changes: 3 additions & 0 deletions codis/config/proxy.toml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not modify the default configuration

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you only pull what is needed?

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ admin_addr = "0.0.0.0:11080"

# Set bind address for proxy, proto_type can be "tcp", "tcp4", "tcp6", "unix" or "unixpacket".
proto_type = "tcp4"
#proxy_tls = true
#proxy_tls_cert = "/usr/local/src/pika/codis/cert.pem"
#proxy_tls_key = "/usr/local/src/pika/codis/key.pem"
proxy_addr = "0.0.0.0:19000"

# Set jodis address & session timeout
Expand Down
3 changes: 3 additions & 0 deletions codis/pkg/models/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ type Proxy struct {

ProtoType string `json:"proto_type"`
ProxyAddr string `json:"proxy_addr"`
ProxyTLS bool `json:"proxy_tls"`
ProxyTLSCert string `json:"proxy_tls_cert"`
ProxyTLSKey string `json:"proxy_tls_key"`

JodisPath string `json:"jodis_path,omitempty"`

Expand Down
19 changes: 19 additions & 0 deletions codis/pkg/models/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,25 @@ func (s *Store) Release() error {
return s.client.Delete(s.LockPath())
}

func (s *Store) ReleaseByToken(token string) error {
b, err := s.client.Read(s.LockPath(),false)
if err != nil {
return err
}

if b != nil {
t, err := Decode(b)
if err != nil {
return err
}
if t != nil && t.Token == token {
return s.Release()
}
}

return nil
}

func (s *Store) LoadTopom(must bool) (*Topom, error) {
return LoadTopom(s.client, s.product, must)
}
Expand Down
8 changes: 8 additions & 0 deletions codis/pkg/models/topom.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,11 @@ type Topom struct {
func (t *Topom) Encode() []byte {
return jsonEncode(t)
}

func Decode(b []byte) (*Topom, error) {
s := &Topom{}
if err := jsonDecode(s, b); err != nil {
return nil, fmt.Errorf("json decode error: %w", err)
}
return s,nil
}
6 changes: 6 additions & 0 deletions codis/pkg/proxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ admin_addr = "0.0.0.0:11080"

# Set bind address for proxy, proto_type can be "tcp", "tcp4", "tcp6", "unix" or "unixpacket".
proto_type = "tcp4"
#proxy_tls = true
#proxy_tls_cert = "/usr/local/src/pika/codis/cert.pem"
#proxy_tls_key = "/usr/local/src/pika/codis/key.pem"
proxy_addr = "0.0.0.0:19000"

# Set jodis address & session timeout
Expand Down Expand Up @@ -166,6 +169,9 @@ type Config struct {

ProxyDataCenter string `toml:"proxy_datacenter" json:"proxy_datacenter"`
ProxyMaxClients int `toml:"proxy_max_clients" json:"proxy_max_clients"`
ProxyTLS bool `toml:"proxy_tls" json:"proxy_tls"`
ProxyTLSCert string `toml:"proxy_tls_cert" json:"proxy_tls_cert"`
ProxyTLSKey string `toml:"proxy_tls_key" json:"proxy_tls_key"`
ProxyMaxOffheapBytes bytesize.Int64 `toml:"proxy_max_offheap_size" json:"proxy_max_offheap_size"`
ProxyHeapPlaceholder bytesize.Int64 `toml:"proxy_heap_placeholder" json:"proxy_heap_placeholder"`

Expand Down
51 changes: 44 additions & 7 deletions codis/pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package proxy

import (
"crypto/tls"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -103,19 +104,55 @@ func New(config *Config) (*Proxy, error) {

func (p *Proxy) setup(config *Config) error {
proto := config.ProtoType
if l, err := net.Listen(proto, config.ProxyAddr); err != nil {
return errors.Trace(err)

var l net.Listener
var err error

if config.ProxyTLS {
log.Printf("TLS configuration: cert = %s, key = %s", config.ProxyTLSCert, config.ProxyTLSKey)

// Load the TLS certificate
cert, err := tls.LoadX509KeyPair(config.ProxyTLSCert, config.ProxyTLSKey)
if err != nil {
return errors.Trace(err)
}

// Set up TLS configuration to use TLS 1.2 or higher
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}

// Create a TLS listener
l, err = tls.Listen(proto, config.ProxyAddr, tlsConfig)
if err != nil {
return errors.Trace(err)
}
} else {
p.lproxy = l
log.Printf("Setting up a non-TLS listener")

x, err := utils.ReplaceUnspecifiedIP(proto, l.Addr().String(), config.HostProxy)
// Create a non-TLS listener
l, err = net.Listen(proto, config.ProxyAddr)
if err != nil {
return err
return errors.Trace(err)
}
p.model.ProtoType = proto
p.model.ProxyAddr = x
}

// Common setup for both TLS and non-TLS
p.lproxy = l

// Replace unspecified IP address with the specific host proxy address
x, err := utils.ReplaceUnspecifiedIP(proto, l.Addr().String(), config.HostProxy)
if err != nil {
log.Errorf("Failed to replace unspecified IP: %s", err)
return err
}

// Update proxy model
p.model.ProtoType = proto
p.model.ProxyAddr = x


proto = "tcp"
if l, err := net.Listen(proto, config.AdminAddr); err != nil {
return errors.Trace(err)
Expand Down
3 changes: 2 additions & 1 deletion codis/pkg/topom/topom.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ func (s *Topom) Close() error {
defer s.store.Close()

if s.online {
if err := s.store.Release(); err != nil {
err := s.store.ReleaseByToken(s.model.Token);
if err != nil {
Comment on lines +172 to +173
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve error handling in Close method.

Ensure that the error is logged and handled appropriately.

	if s.online {
		err := s.store.ReleaseByToken(s.model.Token)
		if err != nil {
			log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
			return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
		}
	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err := s.store.ReleaseByToken(s.model.Token);
if err != nil {
if s.online {
err := s.store.ReleaseByToken(s.model.Token)
if err != nil {
log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
}
}

log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
}
Expand Down
Loading