Skip to content

Commit

Permalink
various fixes regarding enabling/disabling/validation of jetstream on…
Browse files Browse the repository at this point in the history
… system account

[FIX] added check for --js-enable=tier on system account (#685)
[FIX] added bypass to allow --js-disable to work on system account in cases where it was enabled by specifying --js-enable=tier
[FIX] enhanced validation to check for system accounts with enabled jetstream
  • Loading branch information
aricart authored Dec 23, 2024
1 parent ad42dfb commit 296ba90
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 3 deletions.
7 changes: 6 additions & 1 deletion cmd/editaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,12 @@ func (p *EditAccountParams) checkSystemAccount(ctx ActionCtx) error {
return nil
}

if p.claim.Limits.JetStreamTieredLimits != nil {
// allow the js to be disabled
if p.disableJetStream {
return nil
}

if p.claim.Limits.JetStreamTieredLimits != nil || p.enableJetStream > -1 {
return errors.New("system account cannot have JetStream limits - please rerun with --js-disable")
}

Expand Down
52 changes: 52 additions & 0 deletions cmd/editaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,3 +524,55 @@ func Test_EnableTierNoOtherFlag(t *testing.T) {
require.Error(t, err)
require.Equal(t, "rm-js-tier is exclusive of all other js options", err.Error())
}

func Test_CannotEnableJsInSys(t *testing.T) {
ts := NewTestStore(t, "O")
defer ts.Done(t)
ts.AddAccount(t, "SYS")
_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
require.NoError(t, err)

_, _, err = ExecuteCmd(createEditAccount(), "--js-enable", "1")
require.Error(t, err)

_, _, err = ExecuteCmd(createEditAccount(), "--js-disable")
require.NoError(t, err)

sys, err := ts.Store.ReadAccountClaim("SYS")
require.NoError(t, err)

require.False(t, sys.Limits.IsJSEnabled())
}

func Test_AllowSysToDisableJs(t *testing.T) {
ts := NewTestStore(t, "O")
defer ts.Done(t)
ts.AddAccount(t, "SYS")
_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
require.NoError(t, err)

sys, err := ts.Store.ReadAccountClaim("SYS")
require.NoError(t, err)
require.False(t, sys.Limits.IsJSEnabled())

sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits)
sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1}

okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t))
require.NoError(t, err)
token, err := sys.Encode(okp)
require.NoError(t, err)
require.NoError(t, ts.Store.StoreRaw([]byte(token)))

sys, err = ts.Store.ReadAccountClaim("SYS")
require.NoError(t, err)
require.True(t, sys.Limits.IsJSEnabled())

_, _, err = ExecuteCmd(createEditAccount(), "--js-disable")
require.NoError(t, err)

sys, err = ts.Store.ReadAccountClaim("SYS")
require.NoError(t, err)

require.False(t, sys.Limits.IsJSEnabled())
}
10 changes: 9 additions & 1 deletion cmd/validate.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018-2019 The NATS Authors
* Copyright 2018-2024 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -220,6 +220,14 @@ func (p *ValidateCmdParams) validate(ctx ActionCtx) error {
if aci != nil {
p.accountValidations[v] = aci
}
if oc.SystemAccount == ac.Subject {
if ac.Limits.IsJSEnabled() {
if p.accountValidations[v] == nil {
p.accountValidations[v] = &jwt.ValidationResults{}
}
p.accountValidations[v].AddError("JetStream should not be enabled for system account")
}
}
if !oc.DidSign(ac) {
if p.accountValidations[v] == nil {
p.accountValidations[v] = &jwt.ValidationResults{}
Expand Down
28 changes: 27 additions & 1 deletion cmd/validate_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018-2023 The NATS Authors
* Copyright 2018-2024 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand All @@ -21,6 +21,7 @@ import (
"strings"
"testing"

"github.com/nats-io/jwt/v2"
"github.com/nats-io/nsc/v2/cmd/store"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -201,3 +202,28 @@ func Test_ValidateInteractive(t *testing.T) {
require.NoError(t, err)
require.Contains(t, stderr, "Account \"B\"")
}

func Test_ValidateJsSys(t *testing.T) {
ts := NewTestStore(t, "O")
defer ts.Done(t)
ts.AddAccount(t, "SYS")
_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
require.NoError(t, err)

sys, err := ts.Store.ReadAccountClaim("SYS")
require.NoError(t, err)
require.False(t, sys.Limits.IsJSEnabled())

sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits)
sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1}

okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t))
require.NoError(t, err)
token, err := sys.Encode(okp)
require.NoError(t, err)
require.NoError(t, ts.Store.StoreRaw([]byte(token)))

_, stderr, err := ExecuteInteractiveCmd(HoistRootFlags(createValidateCommand()), []interface{}{1})
require.Error(t, err)
require.Contains(t, stderr, "JetStream should not be enabled for system account")
}

0 comments on commit 296ba90

Please sign in to comment.