Skip to content

Commit

Permalink
[#956] Fix broker property with ordinal parser
Browse files Browse the repository at this point in the history
  • Loading branch information
@brusdev @gtully
brusdev authored and gtully committed Jun 14, 2024
1 parent c7284a9 commit dd7e466
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 38 deletions.
68 changes: 30 additions & 38 deletions controllers/activemqartemis_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2436,7 +2436,7 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) addResourceForBrokerProperties(
desired = obj.(*corev1.Secret)
}

data := brokerPropertiesData(customResource.Spec.BrokerProperties)
data := BrokerPropertiesData(customResource.Spec.BrokerProperties)

if desired == nil {
reconciler.log.V(1).Info("desired brokerprop secret nil, create new one", "name", resourceName.Name)
Expand Down Expand Up @@ -2467,41 +2467,33 @@ func alder32Of(props []string) []byte {
return digest.Sum(nil)
}

func brokerPropertiesData(props []string) map[string]string {
func BrokerPropertiesData(props []string) map[string]string {
contents := make(map[string]string)

buf := &bytes.Buffer{}
fmt.Fprintln(buf, "# generated by crd")
fmt.Fprintln(buf, "#")

hasOrdinalPrefix := false
for _, propertyKeyVal := range props {
if strings.HasPrefix(propertyKeyVal, OrdinalPrefix) && strings.Index(propertyKeyVal, OrdinalPrefixSep) > 0 {
hasOrdinalPrefix = true
continue
}
fmt.Fprintf(buf, "%s\n", propertyKeyVal)
}
matches := ParseBrokerPropertyWithOrdinal(propertyKeyVal)

contents := map[string]string{BrokerPropertiesName: buf.String()}
if len(matches) > 0 {
mapKey := matches[1] + OrdinalPrefixSep + BrokerPropertiesName
value := matches[2]

if hasOrdinalPrefix {
for _, propertyKeyVal := range props {
if strings.HasPrefix(propertyKeyVal, OrdinalPrefix) {
if i := strings.Index(propertyKeyVal, OrdinalPrefixSep); i > 0 {
// use a key that will match the volume projection and broker status
mapKey := propertyKeyVal[:i+len(OrdinalPrefixSep)] + BrokerPropertiesName
value := propertyKeyVal[i+len(OrdinalPrefixSep):]

existing, found := contents[mapKey]
if found {
contents[mapKey] = fmt.Sprintf("%s%s\n", existing, value)
} else {
contents[mapKey] = fmt.Sprintf("%s\n", value)
}
}
existing, found := contents[mapKey]
if found {
contents[mapKey] = fmt.Sprintf("%s%s\n", existing, value)
} else {
contents[mapKey] = fmt.Sprintf("%s\n", value)
}
} else {
fmt.Fprintf(buf, "%s\n", propertyKeyVal)
}
}

contents[BrokerPropertiesName] = buf.String()

return contents
}

Expand Down Expand Up @@ -2637,8 +2629,9 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) createExtraConfigmapsAndSecrets
if secret == brokePropertiesResourceName && len(brokerPropsData) > 1 {
// place ordinal data in subpath in order
for _, key := range sortedKeys(brokerPropsData) {
if hasOrdinal, separatorIndex := extractOrdinalPrefixSeperatorIndex(key); hasOrdinal {
subPath := key[:separatorIndex]
matches := ParseBrokerPropertyWithOrdinal(key)
if len(matches) > 0 {
subPath := matches[1]
secretVol.VolumeSource.Secret.Items = append(secretVol.VolumeSource.Secret.Items, corev1.KeyToPath{Key: key, Path: fmt.Sprintf("%s/%s", subPath, key)})
} else {
secretVol.VolumeSource.Secret.Items = append(secretVol.VolumeSource.Secret.Items, corev1.KeyToPath{Key: key, Path: key})
Expand All @@ -2658,8 +2651,9 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) createExtraConfigmapsAndSecrets

if len(bpSecret.Data) > 0 {
for _, key := range sortedKeysStringKeyByteValue(bpSecret.Data) {
if hasOrdinal, separatorIndex := extractOrdinalPrefixSeperatorIndex(key); hasOrdinal {
subPath := key[:separatorIndex]
matches := ParseBrokerPropertyWithOrdinal(key)
if len(matches) > 0 {
subPath := matches[1]
secretVol.VolumeSource.Secret.Items = append(secretVol.VolumeSource.Secret.Items, corev1.KeyToPath{Key: key, Path: fmt.Sprintf("%s/%s", subPath, key)})
} else {
secretVol.VolumeSource.Secret.Items = append(secretVol.VolumeSource.Secret.Items, corev1.KeyToPath{Key: key, Path: key})
Expand All @@ -2677,15 +2671,13 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) createExtraConfigmapsAndSecrets
return extraVolumes, extraVolumeMounts, nil
}

func extractOrdinalPrefixSeperatorIndex(key string) (bool, int) {

prefixIndex := strings.Index(key, OrdinalPrefix)
separatorIndex := strings.Index(key, OrdinalPrefixSep)
var brokerPropertyWithOrdinalRegex *regexp.Regexp

if prefixIndex == 0 && separatorIndex > len(OrdinalPrefix) {
return true, separatorIndex
func ParseBrokerPropertyWithOrdinal(property string) []string {
if brokerPropertyWithOrdinalRegex == nil {
brokerPropertyWithOrdinalRegex = regexp.MustCompile("^(" + regexp.QuoteMeta(OrdinalPrefix) + "[0-9]+)" + regexp.QuoteMeta(OrdinalPrefixSep) + "(.*)$")
}
return false, -1
return brokerPropertyWithOrdinalRegex.FindStringSubmatch(property)
}

func (reconciler *ActiveMQArtemisReconcilerImpl) NewStatefulSetForCR(customResource *brokerv1beta1.ActiveMQArtemis, namer common.Namers, currentStateFullSet *appsv1.StatefulSet, client rtclient.Client) (*appsv1.StatefulSet, error) {
Expand Down Expand Up @@ -3115,8 +3107,8 @@ func checkProjectionStatus(cr *brokerv1beta1.ActiveMQArtemis, client rtclient.Cl

if !present {
// with ordinal prefix or extras in the map this can be the case
isForOrdinal, _ := extractOrdinalPrefixSeperatorIndex(name)
if !(name == JaasConfigKey || strings.HasPrefix(name, UncheckedPrefix) || isForOrdinal) {
matches := ParseBrokerPropertyWithOrdinal(name)
if !(name == JaasConfigKey || strings.HasPrefix(name, UncheckedPrefix) || len(matches) > 0) {
missingKeys = append(missingKeys, name)
}
continue
Expand Down
89 changes: 89 additions & 0 deletions controllers/activemqartemis_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1350,3 +1350,92 @@ func TestGetBrokerHost(t *testing.T) {
ingressHost = formatTemplatedString(&cr, specIngressHost, "2", "my-console", "abc")
assert.Equal(t, "test-test-ns-my-console-2-abc.my-domain.com", ingressHost)
}

func TestParseBrokerPropertyWithOrdinal(t *testing.T) {
var matches []string

matches = ParseBrokerPropertyWithOrdinal("broker-0.maxDiskUsage")
assert.Equal(t, 3, len(matches))
assert.Equal(t, "broker-0.maxDiskUsage", matches[0])
assert.Equal(t, "broker-0", matches[1])
assert.Equal(t, "maxDiskUsage", matches[2])

matches = ParseBrokerPropertyWithOrdinal("broker-999.maxDiskUsage=97")
assert.Equal(t, 3, len(matches))
assert.Equal(t, "broker-999.maxDiskUsage=97", matches[0])
assert.Equal(t, "broker-999", matches[1])
assert.Equal(t, "maxDiskUsage=97", matches[2])

matches = ParseBrokerPropertyWithOrdinal("maxDiskUsage=97")
assert.Equal(t, 0, len(matches))

matches = ParseBrokerPropertyWithOrdinal("a.broker-0.maxDiskUsage")
assert.Equal(t, 0, len(matches))

matches = ParseBrokerPropertyWithOrdinal("broker-0-maxDiskUsage")
assert.Equal(t, 0, len(matches))

matches = ParseBrokerPropertyWithOrdinal("broker-a.maxDiskUsage")
assert.Equal(t, 0, len(matches))
}

func TestBrokerPropertiesData(t *testing.T) {

data := BrokerPropertiesData([]string{
"maxDiskUsage=97",
"minDiskFree=5",
})

assert.Equal(t, 1, len(data))

assert.True(t, strings.Contains(data[BrokerPropertiesName], "maxDiskUsage=97"))
assert.True(t, strings.Contains(data[BrokerPropertiesName], "minDiskFree=5"))
}

func TestBrokerPropertiesDataWithOrdinal(t *testing.T) {

data := BrokerPropertiesData([]string{
"broker-0.maxDiskUsage=98",
"broker-0.minDiskFree=6",
"broker-999.maxDiskUsage=99",
"broker-999.minDiskFree=7",
})

assert.Equal(t, 3, len(data))

assert.False(t, strings.Contains(data[BrokerPropertiesName], "maxDiskUsage"))
assert.False(t, strings.Contains(data[BrokerPropertiesName], "minDiskFree"))

broker0BrokerPropertiesName := "broker-0" + OrdinalPrefixSep + BrokerPropertiesName
assert.True(t, strings.Contains(data[broker0BrokerPropertiesName], "maxDiskUsage=98"))
assert.True(t, strings.Contains(data[broker0BrokerPropertiesName], "minDiskFree=6"))

broker999BrokerPropertiesName := "broker-999" + OrdinalPrefixSep + BrokerPropertiesName
assert.True(t, strings.Contains(data[broker999BrokerPropertiesName], "maxDiskUsage=99"))
assert.True(t, strings.Contains(data[broker999BrokerPropertiesName], "minDiskFree=7"))
}

func TestBrokerPropertiesDataWithAndWithoutOrdinal(t *testing.T) {

data := BrokerPropertiesData([]string{
"maxDiskUsage=97",
"minDiskFree=5",
"broker-0.maxDiskUsage=98",
"broker-0.minDiskFree=6",
"broker-999.maxDiskUsage=99",
"broker-999.minDiskFree=7",
})

assert.Equal(t, 3, len(data))

assert.True(t, strings.Contains(data[BrokerPropertiesName], "maxDiskUsage=97"))
assert.True(t, strings.Contains(data[BrokerPropertiesName], "minDiskFree=5"))

broker0BrokerPropertiesName := "broker-0" + OrdinalPrefixSep + BrokerPropertiesName
assert.True(t, strings.Contains(data[broker0BrokerPropertiesName], "maxDiskUsage=98"))
assert.True(t, strings.Contains(data[broker0BrokerPropertiesName], "minDiskFree=6"))

broker999BrokerPropertiesName := "broker-999" + OrdinalPrefixSep + BrokerPropertiesName
assert.True(t, strings.Contains(data[broker999BrokerPropertiesName], "maxDiskUsage=99"))
assert.True(t, strings.Contains(data[broker999BrokerPropertiesName], "minDiskFree=7"))
}

0 comments on commit dd7e466

Please sign in to comment.