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

Fix NatsRegistry #8281

Merged
merged 1 commit into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-nats-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Fix nats registry

The nats registry would behave badly when configuring `nats-js-kv` via envvar. Reason is the way go-micro initializes.
It took 5 developers to find the issue and the fix so the details cannot be shared here. Just accept that it is working now
Copy link
Contributor

Choose a reason for hiding this comment

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

🙈


https://github.com/owncloud/ocis/pull/8281
11 changes: 0 additions & 11 deletions ocis-pkg/natsjsregistry/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

type storeOptionsKey struct{}
type expiryKey struct{}
type authKey struct{}

// StoreOptions sets the options for the underlying store
func StoreOptions(opts []store.Option) registry.Option {
Expand All @@ -31,13 +30,3 @@ func ServiceExpiry(t time.Duration) registry.Option {
o.Context = context.WithValue(o.Context, expiryKey{}, t)
}
}

// Authenticate sets the username/password for the nats connection
func Authenticate(username, password string) registry.Option {
return func(o *registry.Options) {
if o.Context == nil {
o.Context = context.Background()
}
o.Context = context.WithValue(o.Context, authKey{}, []string{username, password})
}
}
38 changes: 29 additions & 9 deletions ocis-pkg/natsjsregistry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"context"
"encoding/json"
"errors"
"os"
"strings"
"time"

natsjskv "github.com/go-micro/plugins/v4/store/nats-js-kv"
Expand All @@ -14,7 +16,12 @@ import (
"go-micro.dev/v4/util/cmd"
)

var _registryName = "nats-js-kv"
var (
_registryName = "nats-js-kv"
_registryAddressEnv = "MICRO_REGISTRY_ADDRESS"
_registryUsernameEnv = "MICRO_REGISTRY_AUTH_USERNAME"
_registryPasswordEnv = "MICRO_REGISTRY_AUTH_PASSWORD"
)

func init() {
cmd.DefaultRegistries[_registryName] = NewRegistry
Expand All @@ -31,7 +38,7 @@ func NewRegistry(opts ...registry.Option) registry.Registry {
exp, _ := options.Context.Value(expiryKey{}).(time.Duration)
return &storeregistry{
opts: options,
store: natsjskv.NewStore(append(storeOptions(options), natsjskv.DefaultMemory())...),
store: natsjskv.NewStore(storeOptions(options)...),
typ: _registryName,
expiry: exp,
}
Expand Down Expand Up @@ -127,18 +134,31 @@ func (n *storeregistry) String() string {

func storeOptions(opts registry.Options) []store.Option {
storeoptions := []store.Option{
store.Nodes(opts.Addrs...),
store.Database("service-registry"),
store.Table("service-registry"),
natsjskv.DefaultMemory(),
}
if so, ok := opts.Context.Value(storeOptionsKey{}).([]store.Option); ok {
storeoptions = append(storeoptions, so...)

addr := []string{"127.0.0.1:9233"}
if len(opts.Addrs) > 0 {
addr = opts.Addrs
} else if a := strings.Split(os.Getenv(_registryAddressEnv), ","); len(a) > 0 && a[0] != "" {
addr = a
}
storeoptions = append(storeoptions, store.Nodes(addr...))

natsOptions := nats.GetDefaultOptions()
natsOptions.Name = "nats-js-kv-registry"
if auth, ok := opts.Context.Value(authKey{}).([]string); ok {
natsOptions.User = auth[0]
natsOptions.Password = auth[1]
natsOptions.User, natsOptions.Password = getAuth()
storeoptions = append(storeoptions, natsjskv.NatsOptions(natsOptions))

if so, ok := opts.Context.Value(storeOptionsKey{}).([]store.Option); ok {
storeoptions = append(storeoptions, so...)
}
return append(storeoptions, natsjskv.NatsOptions(natsOptions))

return storeoptions
}

func getAuth() (string, string) {
return os.Getenv(_registryUsernameEnv), os.Getenv(_registryPasswordEnv)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker but ugly. I am aware that it only works this way :/

}
13 changes: 4 additions & 9 deletions ocis-pkg/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import (
)

const (
registryEnv = "MICRO_REGISTRY"
registryAddressEnv = "MICRO_REGISTRY_ADDRESS"
registryUsernameEnv = "MICRO_REGISTRY_AUTH_USERNAME"
registryPasswordEnv = "MICRO_REGISTRY_AUTH_PASSWORD"
_registryEnv = "MICRO_REGISTRY"
_registryAddressEnv = "MICRO_REGISTRY_ADDRESS"
)

var (
Expand Down Expand Up @@ -64,7 +62,6 @@ func GetRegistry(opts ...Option) mRegistry.Registry {
case "natsjs", "nats-js", "nats-js-kv": // for backwards compatibility - we will stick with one of those
_reg = natsjsregistry.NewRegistry(
mRegistry.Addrs(cfg.Addresses...),
natsjsregistry.Authenticate(cfg.Username, cfg.Password),
)
case "memory":
_reg = memr.NewRegistry()
Expand Down Expand Up @@ -112,15 +109,13 @@ func getEnvs(opts ...Option) *Config {
cfg := &Config{
Type: "nats-js-kv",
Addresses: []string{"127.0.0.1:9233"},
Username: os.Getenv(registryUsernameEnv),
Password: os.Getenv(registryPasswordEnv),
}

if s := os.Getenv(registryEnv); s != "" {
if s := os.Getenv(_registryEnv); s != "" {
cfg.Type = s
}

if s := strings.Split(os.Getenv(registryAddressEnv), ","); len(s) > 0 && s[0] != "" {
if s := strings.Split(os.Getenv(_registryAddressEnv), ","); len(s) > 0 && s[0] != "" {
cfg.Addresses = s
}

Expand Down