Skip to content

Commit

Permalink
allow for empty security context when security is enabled
Browse files Browse the repository at this point in the history
convert rack-id to int64 rather than int
make compare to default more robust
  • Loading branch information
Jesse Schmidt committed Dec 5, 2023
1 parent 429c121 commit 77130b6
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 14 deletions.
60 changes: 51 additions & 9 deletions asconfig/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (s *copyEffectiveRackIDStep) execute(conf Conf) error {
return fmt.Errorf("unable to find rack id for rack %s", rack)
}

rackID, err := strconv.Atoi(rackIDStr)
rackID, err := strconv.ParseInt(rackIDStr, 10, 64) // Matches what is found in info/as_parser.go

if err != nil {
return fmt.Errorf("unable to convert rack id %s to int", rackIDStr)
Expand Down Expand Up @@ -612,10 +612,10 @@ func newRemoveSecurityIfDisabledStep(log logr.Logger) *removeSecurityIfDisabledS
func (s *removeSecurityIfDisabledStep) execute(conf Conf) error {
s.log.V(1).Info("Removing security configs if security is disabled")

contexts := conf["flat_config"].(Conf)
flatConf := conf["flat_config"].(Conf)
build := conf["metadata"].(Conf)["asd_build"].(string)

if val, ok := contexts["security.enable-security"]; ok {
if val, ok := flatConf["security.enable-security"]; ok {
securityEnabled, ok := val.(bool)

if !ok {
Expand All @@ -629,15 +629,15 @@ func (s *removeSecurityIfDisabledStep) execute(conf Conf) error {

if securityEnabled {
if cmp >= 0 {
delete(contexts, "security.enable-security")
delete(flatConf, "security.enable-security")
}
} else {
// 5.7 and newer can't have any security configs. An empty security
// context will enable-security.
if cmp >= 0 {
for key := range contexts {
for key := range flatConf {
if securityRe.MatchString(key) {
delete(contexts, key)
delete(flatConf, key)
}
}
}
Expand Down Expand Up @@ -680,13 +680,33 @@ func compareDefaults(defVal, confVal interface{}) bool {
return defVal == confVal
case uint64:
// Schema deals with uint64 when positive but config deals with int
return val == uint64(confVal.(int))
switch confVal := confVal.(type) {
case int64:
if confVal < 0 {
return false
}

return val == uint64(confVal)
case uint64:
return val == confVal
}
case int64:
// Schema deals with int64 when negative but config deals with int
return val == int64(confVal.(int))
switch confVal := confVal.(type) {
case uint64:
if val < 0 {
return false
}

return val == int64(confVal)
case int64:
return val == confVal
}
default:
return val == confVal
}

return false
}

func defaultSlice(m map[string][]string, key string) []string {
Expand Down Expand Up @@ -717,10 +737,17 @@ func (s *removeDefaultsStep) execute(conf Conf) error {
// We will use this to find the most common log level in order to replace
// with "any".
loggingMap := make(map[string]map[string][]string)
securityFound := false

for key, value := range flatConf {
if strings.HasPrefix(key, "security"+sep) {
// We expect there to be no security keys if security is disabled in
// 5.7 or newer
securityFound = true
}

// Handle logging differently
if strings.HasPrefix(key, "logging.") {
if strings.HasPrefix(key, "logging"+sep) {
contextKey, _ := splitContextBaseKey(key)

if loggingMap[contextKey] == nil {
Expand Down Expand Up @@ -783,6 +810,21 @@ func (s *removeDefaultsStep) execute(conf Conf) error {
flatConf[loggingContext+sep+"any"] = freq
}

if securityFound {
build := conf["metadata"].(Conf)["asd_build"].(string)
cmp, err := lib.CompareVersions(build, "5.7.0")

if err != nil {
return err
}

if cmp >= 0 {
// Security is enabled and we are 5.7 or newer. This ensures there
// is at least an empry security context.
flatConf["security"] = make(Conf)
}
}

return nil
}

Expand Down
47 changes: 43 additions & 4 deletions asconfig/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,25 @@ type GenerateTC struct {
expected Conf
}

func convertIntToInt64(conf Conf) Conf {
for key, value := range conf {
switch v := value.(type) {
case int:
conf[key] = int64(v)
case Conf:
conf[key] = convertIntToInt64(v)
case []Conf:
for i, c := range v {
v[i] = convertIntToInt64(c)
}
}
}
return conf
}

func (suite *GenerateTestSuite) TestGenerate() {
testCases := []GenerateTC{
logging,
loggingTC,
namespaceTC,
networkTC,
serviceTC,
Expand All @@ -44,21 +60,22 @@ func (suite *GenerateTestSuite) TestGenerate() {
networkDefaultsTC,
serviceDefaultsTC,
security57DefaultsTC,
security57AllDefaultsTC,
xdr5DefaultsTC,
}

InitFromMap(logr.Discard(), testSchemas)

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.mockGetter.EXPECT().AllConfigs().Return(tc.allConfigs, nil)
suite.mockGetter.EXPECT().AllConfigs().Return(convertIntToInt64(tc.allConfigs), nil)
suite.mockGetter.EXPECT().GetAsInfo("metadata").Return(tc.metadata, nil)
logger := logr.Discard()

actual, err := GenerateConf(logger, suite.mockGetter, tc.removeDefaults)

suite.Assert().Nil(err)
suite.Assert().Equal(tc.expected, actual)
suite.Assert().Equal(convertIntToInt64(tc.expected), actual)
})
}
}
Expand All @@ -67,7 +84,7 @@ func TestGenerateTestSuiteSuite(t *testing.T) {
suite.Run(t, new(GenerateTestSuite))
}

var logging = GenerateTC{
var loggingTC = GenerateTC{
"logging",
false,
Conf{
Expand Down Expand Up @@ -1911,6 +1928,28 @@ var security57DefaultsTC = GenerateTC{
},
}

var security57AllDefaultsTC = GenerateTC{
"security post 5.7 with remove defaults",
true,
Conf{
"security": Conf{
"enable-quotas": false,
"enable-security": true,
"log.report-authentication": false,
"log.report-sys-admin": false,
"log.report-user-admin": false,
"log.report-violation": false,
"privilege-refresh-period": 300,
"session-ttl": 86400,
"tps-weight": 2,
},
},
Conf{"metadata": Conf{"asd_build": "6.4.0.0", "node_id": "BB9030011AC4202"}},
Conf{
"security": Conf{},
},
}

var serviceDefaultsTC = GenerateTC{
"service with remove defaults",
true,
Expand Down
9 changes: 8 additions & 1 deletion asconfig/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,14 @@ func expandConf(log logr.Logger, input *Conf, sep string) Conf {
func expandConfMap(log logr.Logger, input *Conf, sep string) Conf {
m := make(Conf)

for k, v := range *input {
// generate.go adds "security": Conf{} to flatMap to ensure that an empty
// security context is present in the config. This is required to enable
// security in server >= 5.7. Sorting the keys ensures "security" is process
// before "security.*" keys.
keys := sortKeys(*input)

for _, k := range keys {
v := (*input)[k]
switch v := v.(type) {
case Conf:
m[k] = expandConfMap(log, &v, sep)
Expand Down

0 comments on commit 77130b6

Please sign in to comment.