feat(update-security-json): Update security.json when secret is updated

Author: mcarroll1

api/v1beta1/solrcloud_types.go

@@ -19,12 +19,13 @@ package v1beta1
 
 import (
 	"fmt"
-	"github.com/go-logr/logr"
-	zkApi "github.com/pravega/zookeeper-operator/api/v1beta1"
 	"math/rand"
 	"strconv"
 	"strings"
 
+	"github.com/go-logr/logr"
+	zkApi "github.com/pravega/zookeeper-operator/api/v1beta1"
+
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	corev1 "k8s.io/api/core/v1"
@@ -681,7 +682,8 @@ type ManagedUpdateOptions struct {
 	// The maximum number of pods that can be unavailable during the update.
 	// Value can be an absolute number (ex: 5) or a percentage of the desired number of pods (ex: 10%).
 	// Absolute number is calculated from percentage by rounding down.
-	// If the provided number is 0 or negative, then all pods will be allowed to be updated in unison.
+	// If the provided number is 0, then all pods will be allowed to be updated in unison.
+	// Negatives are not allowed.
 	//
 	// Defaults to 25%.
 	//
@@ -691,7 +693,8 @@ type ManagedUpdateOptions struct {
 	// The maximum number of replicas for each shard that can be unavailable during the update.
 	// Value can be an absolute number (ex: 5) or a percentage of replicas in a shard (ex: 25%).
 	// Absolute number is calculated from percentage by rounding down.
-	// If the provided number is 0 or negative, then all replicas will be allowed to be updated in unison.
+	// If the provided number is 0 , then all replicas will be allowed to be updated in unison.
+	// Negatives are not allowed.
 	//
 	// Defaults to 1.
 	//
@@ -1621,6 +1624,15 @@ const (
 	Basic AuthenticationType = "Basic"
 )
 
+type BootstrapSecurityJson struct {
+	SecurityJsonSecret *corev1.SecretKeySelector `json:"bootstrapSecurityJson,omitempty"`
+
+	// Flag to indicate if the operator should overwrite an existing security.json if there are changes
+	// as compared to the underlying secret
+	// +optional
+	Overwrite bool `json:"probesRequireAuth,omitempty"`
+}
+
 type SolrSecurityOptions struct {
 	// Indicates the authentication plugin type that is being used by Solr; for now only "Basic" is supported by the
 	// Solr operator but support for other authentication plugins may be added in the future.
@@ -1649,7 +1661,5 @@ type SolrSecurityOptions struct {
 
 	// Configure a user-provided security.json from a secret to allow for advanced security config.
 	// If not specified, the operator bootstraps a security.json with basic auth enabled.
-	// This is a bootstrapping config only; once Solr is initialized, the security config should be managed by the security API.
-	// +optional
-	BootstrapSecurityJson *corev1.SecretKeySelector `json:"bootstrapSecurityJson,omitempty"`
+	BootstrapSecurityJson *BootstrapSecurityJson `json:"bootstrapSecurityJson,omitempty"`
 }

api/v1beta1/zz_generated.deepcopy.go

@@ -10205,29 +10205,34 @@ spec:
                     description: |-
                       Configure a user-provided security.json from a secret to allow for advanced security config.
                       If not specified, the operator bootstraps a security.json with basic auth enabled.
-                      This is a bootstrapping config only; once Solr is initialized, the security config should be managed by the security API.
                     properties:
-                      key:
-                        description: The key of the secret to select from.  Must be
-                          a valid secret key.
-                        type: string
-                      name:
-                        default: ""
+                      bootstrapSecurityJson:
+                        description: SecretKeySelector selects a key of a Secret.
+                        properties:
+                          key:
+                            description: The key of the secret to select from.  Must
+                              be a valid secret key.
+                            type: string
+                          name:
+                            description: |-
+                              Name of the referent.
+                              More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                              TODO: Add other useful fields. apiVersion, kind, uid?
+                            type: string
+                          optional:
+                            description: Specify whether the Secret or its key must
+                              be defined
+                            type: boolean
+                        required:
+                        - key
+                        type: object
+                        x-kubernetes-map-type: atomic
+                      overwrite:
                         description: |-
-                          Name of the referent.
-                          This field is effectively required, but due to backwards compatibility is
-                          allowed to be empty. Instances of this type with an empty value here are
-                          almost certainly wrong.
-                          More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
-                        type: string
-                      optional:
-                        description: Specify whether the Secret or its key must be
-                          defined
+                          Flag to indicate if the operator should overwrite an existing security.json if there are changes
+                          as compared to the underlying secret
                         type: boolean
-                    required:
-                    - key
                     type: object
-                    x-kubernetes-map-type: atomic
                   probesRequireAuth:
                     description: |-
                       Flag to indicate if the configured HTTP endpoint(s) used for the probes require authentication; defaults
@@ -10428,7 +10433,8 @@ spec:
                           The maximum number of pods that can be unavailable during the update.
                           Value can be an absolute number (ex: 5) or a percentage of the desired number of pods (ex: 10%).
                           Absolute number is calculated from percentage by rounding down.
-                          If the provided number is 0 or negative, then all pods will be allowed to be updated in unison.
+                          If the provided number is 0, then all pods will be allowed to be updated in unison.
+                          Negatives are not allowed.
 
                           Defaults to 25%.
                         x-kubernetes-int-or-string: true
@@ -10440,7 +10446,8 @@ spec:
                           The maximum number of replicas for each shard that can be unavailable during the update.
                           Value can be an absolute number (ex: 5) or a percentage of replicas in a shard (ex: 25%).
                           Absolute number is calculated from percentage by rounding down.
-                          If the provided number is 0 or negative, then all replicas will be allowed to be updated in unison.
+                          If the provided number is 0 , then all replicas will be allowed to be updated in unison.
+                          Negatives are not allowed.
 
                           Defaults to 1.
                         x-kubernetes-int-or-string: true

config/crd/bases/solr.apache.org_solrclouds.yaml

@@ -177,9 +177,12 @@ var _ = FDescribe("SolrCloud controller - Basic Auth", func() {
 			solrCloud.Spec.SolrSecurity = &solrv1beta1.SolrSecurityOptions{
 				AuthenticationType: solrv1beta1.Basic,
 				BasicAuthSecret:    basicAuthSecretName,
-				BootstrapSecurityJson: &corev1.SecretKeySelector{
-					LocalObjectReference: corev1.LocalObjectReference{Name: "my-security-json"},
-					Key:                  util.SecurityJsonFile,
+				BootstrapSecurityJson: &solrv1beta1.BootstrapSecurityJson{
+					SecurityJsonSecret: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "my-security-json"},
+						Key:                  util.SecurityJsonFile,
+					},
+					Overwrite: false,
 				},
 			}
 		})
@@ -324,10 +327,11 @@ func expectBasicAuthConfigOnPodTemplateWithGomega(g Gomega, solrCloud *solrv1bet
 			// if the zookeeperRef has ACLs set, verify the env vars were set correctly for this initContainer
 			allACL, _ := solrCloud.Spec.ZookeeperRef.GetACLs()
 			if allACL != nil {
-				g.Expect(expInitContainer.Env).To(HaveLen(10), "Wrong number of env vars using ACLs and Basic Auth")
-				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-2].Name).To(Equal("SOLR_OPTS"), "Env var SOLR_OPTS is misplaced the Solr Pod env vars")
-				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-1].Name).To(Equal("SECURITY_JSON"), "Env var SECURITY_JSON is misplaced the Solr Pod env vars")
-				testACLEnvVarsWithGomega(g, expInitContainer.Env[3:len(expInitContainer.Env)-2], true)
+				g.Expect(expInitContainer.Env).To(HaveLen(11), "Wrong number of env vars using ACLs and Basic Auth")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-3].Name).To(Equal("SOLR_OPTS"), "Env var SOLR_OPTS is misplaced the Solr Pod env vars")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-2].Name).To(Equal("SECURITY_JSON"), "Env var SECURITY_JSON is misplaced the Solr Pod env vars")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-1].Name).To(Equal("SECURITY_JSON_OVERWRITE"), "Env var SECURITY_JSON_OVERWRITE is misplaced the Solr Pod env vars")
+				testACLEnvVarsWithGomega(g, expInitContainer.Env[3:len(expInitContainer.Env)-3], true)
 			} // else this ref not using ACLs
 
 			expectPutSecurityJsonInZkCmd(g, expInitContainer)
@@ -353,9 +357,15 @@ func expectPutSecurityJsonInZkCmd(g Gomega, expInitContainer *corev1.Container)
 	g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the setup-zk InitContainer in the sts!")
 	expCmd := "solr zk cp zk:/security.json /tmp/current_security.json >/dev/null 2>&1;  " +
 		"GET_CURRENT_SECURITY_JSON_EXIT_CODE=$?; if [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 0 ]; then " +
-		"if [ ! -s /tmp/current_security.json ] || grep -q '^{}$' /tmp/current_security.json ]; then  " +
+		"if [ ! -s /tmp/current_security.json ] || grep -q '^{}$' /tmp/current_security.json; then  " +
 		"echo $SECURITY_JSON > /tmp/security.json; solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +
-		" echo 'Blank security.json found. Put new security.json in ZK'; fi; elif [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 1 ]; then " +
+		" echo 'Blank security.json found. Put new security.json in ZK'; " +
+		"elif [ \"${SECURITY_JSON_OVERWRITE}\" = true ] && [ \"$(cat /tmp/current_security.json)\" != \"$(echo $SECURITY_JSON)\" ]; then " +
+		" echo $SECURITY_JSON > /tmp/security.json; solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +
+		" echo 'Diff found. Overwriting security.json in ZK'; " +
+		" else " +
+		" echo 'Not overwriting security.json'; fi; " +
+		"elif [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 1 ]; then " +
 		" echo $SECURITY_JSON > /tmp/security.json; solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +
 		" echo 'No security.json found. Put new security.json in ZK'; fi"
 	g.Expect(expInitContainer.Command[2]).To(ContainSubstring(expCmd), "setup-zk initContainer not configured to bootstrap security.json!")

controllers/solrcloud_controller_basic_auth_test.go

@@ -937,7 +937,7 @@ func expectZkSetupInitContainerForTLSWithGomega(g Gomega, solrCloud *solrv1beta1
 			g.Expect(zkSetupInitContainer.Command[2]).To(ContainSubstring(expChrootCmd), "ZK Setup command does init the chroot")
 			expNumVars := 3
 			if solrCloud.Spec.SolrSecurity != nil && solrCloud.Spec.SolrSecurity.BasicAuthSecret == "" {
-				expNumVars = 4 // one more for SECURITY_JSON
+				expNumVars = 5 // two more for SECURITY_JSON and SECURITY_JSON_OVERWRITE
 			}
 			g.Expect(zkSetupInitContainer.Env).To(HaveLen(expNumVars), "Wrong number of envVars for zk-setup init container")
 		}

controllers/solrcloud_controller_tls_test.go

@@ -23,19 +23,20 @@ import (
 	b64 "encoding/base64"
 	"encoding/json"
 	"fmt"
+	"math/rand"
+	"regexp"
+	"strings"
+	"time"
+
 	solr "github.com/apache/solr-operator/api/v1beta1"
 	"github.com/apache/solr-operator/controllers/util/solr_api"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"math/rand"
-	"regexp"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"strings"
-	"time"
 )
 
 const (
@@ -49,10 +50,11 @@ const (
 // Utility struct holding security related config and objects resolved at runtime needed during reconciliation,
 // such as the secret holding credentials the operator should use to make calls to secure Solr
 type SecurityConfig struct {
-	SolrSecurity      *solr.SolrSecurityOptions
-	CredentialsSecret *corev1.Secret
-	SecurityJson      string
-	SecurityJsonSrc   *corev1.EnvVarSource
+	SolrSecurity          *solr.SolrSecurityOptions
+	CredentialsSecret     *corev1.Secret
+	SecurityJson          string
+	SecurityJsonSrc       *corev1.EnvVarSource
+	SecurityJsonOverwrite bool
 }
 
 // Given a SolrCloud instance and an API service client, produce a SecurityConfig needed to enable Solr security
@@ -117,6 +119,7 @@ func reconcileForBasicAuthWithBootstrappedSecurityJson(ctx context.Context, clie
 
 		// supply the bootstrap security.json to the initContainer via a simple BASE64 encoding env var
 		security.SecurityJson = string(bootstrapSecret.Data[SecurityJsonFile])
+		security.SecurityJsonOverwrite = false
 		basicAuthSecret = authSecret
 	}
 
@@ -136,6 +139,7 @@ func reconcileForBasicAuthWithBootstrappedSecurityJson(ctx context.Context, clie
 		} else {
 			// stash this so we can configure the setup-zk initContainer to bootstrap the security.json in ZK
 			security.SecurityJson = string(bootstrapSecret.Data[SecurityJsonFile])
+			security.SecurityJsonOverwrite = false
 			security.SecurityJsonSrc = &corev1.EnvVarSource{
 				SecretKeyRef: &corev1.SecretKeySelector{
 					LocalObjectReference: corev1.LocalObjectReference{Name: bootstrapSecret.Name}, Key: SecurityJsonFile}}
@@ -166,13 +170,14 @@ func reconcileForBasicAuthWithUserProvidedSecret(ctx context.Context, client *cl
 
 	// is there a user-provided security.json in a secret?
 	// in this config, we don't need to enforce the user providing a security.json as they can bootstrap the security.json however they want
-	if sec.BootstrapSecurityJson != nil {
-		securityJson, err := loadSecurityJsonFromSecret(ctx, client, sec.BootstrapSecurityJson, instance.Namespace)
+	if sec.BootstrapSecurityJson != nil && sec.BootstrapSecurityJson.SecurityJsonSecret != nil {
+		securityJson, err := loadSecurityJsonFromSecret(ctx, client, sec.BootstrapSecurityJson.SecurityJsonSecret, instance.Namespace)
 		if err != nil {
 			return nil, err
 		}
 		security.SecurityJson = securityJson
-		security.SecurityJsonSrc = &corev1.EnvVarSource{SecretKeyRef: sec.BootstrapSecurityJson}
+		security.SecurityJsonSrc = &corev1.EnvVarSource{SecretKeyRef: sec.BootstrapSecurityJson.SecurityJsonSecret}
+		security.SecurityJsonOverwrite = sec.BootstrapSecurityJson.Overwrite
 	} // else no user-provided secret, no sweat for us
 
 	return security, nil
@@ -240,11 +245,16 @@ func cmdToPutSecurityJsonInZk() string {
 	cmd := " solr zk cp zk:/security.json /tmp/current_security.json >/dev/null 2>&1; " +
 		" GET_CURRENT_SECURITY_JSON_EXIT_CODE=$?; " +
 		"if [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 0 ]; then " + // JSON already exists
-		"if [ ! -s /tmp/current_security.json ] || grep -q '^{}$' /tmp/current_security.json ]; then " + // File doesn't exist, is empty, or is just '{}'
+		"if [ ! -s /tmp/current_security.json ] || grep -q '^{}$' /tmp/current_security.json; then " + // File doesn't exist, is empty, or is just '{}'
 		" echo $SECURITY_JSON > /tmp/security.json;" +
 		" solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +
 		" echo 'Blank security.json found. Put new security.json in ZK'; " +
-		"fi; " + // TODO: Consider checking a diff and still applying over the top
+		"elif [ \"${SECURITY_JSON_OVERWRITE}\" = true ] && [ \"$(cat /tmp/current_security.json)\" != \"$(echo $SECURITY_JSON)\" ]; then " + // We want to overwrite the security config if there's a diff
+		" echo $SECURITY_JSON > /tmp/security.json;" +
+		" solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +
+		" echo 'Diff found. Overwriting security.json in ZK'; " +
+		" else " +
+		" echo 'Not overwriting security.json'; fi; " +
 		"elif [ ${GET_CURRENT_SECURITY_JSON_EXIT_CODE} -eq 1 ]; then " + // JSON doesn't exist, but not other error types
 		" echo $SECURITY_JSON > /tmp/security.json;" +
 		" solr zk cp /tmp/security.json zk:/security.json >/dev/null 2>&1; " +

controllers/util/solr_security_util.go

@@ -19,6 +19,10 @@ package util
 
 import (
 	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+
 	solr "github.com/apache/solr-operator/api/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -28,9 +32,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/pointer"
 	"k8s.io/utils/ptr"
-	"sort"
-	"strconv"
-	"strings"
 )
 
 const (
@@ -1263,6 +1264,7 @@ func generateZKInteractionInitContainer(solrCloud *solr.SolrCloud, solrCloudStat
 
 	if security != nil && security.SecurityJson != "" {
 		envVars = append(envVars, corev1.EnvVar{Name: "SECURITY_JSON", ValueFrom: security.SecurityJsonSrc})
+		envVars = append(envVars, corev1.EnvVar{Name: "SECURITY_JSON_OVERWRITE", Value: strconv.FormatBool(security.SecurityJsonOverwrite)})
 		if solrCloud.Spec.SolrZkOpts != "" {
 			envVars = append(envVars, corev1.EnvVar{Name: "ZKCLI_JVM_FLAGS", Value: solrCloud.Spec.SolrZkOpts})
 		}

controllers/util/solr_util.go

@@ -1126,9 +1126,12 @@ spec:
     bootstrapSecurityJson:
       name: my-custom-security-json
       key: security.json
+      overwrite: false
 ```
 For `Basic` authentication, if you don't supply a `security.json` Secret, then the operator assumes you are bootstrapping the security configuration via some other means.
 
+If `overwrite` is set to `true`, the security.json for the cluster will be updated if there is a difference between the underlying secret and the security.json in ZK.
+
 Refer to the example `security.json` shown in the Authorization section above to help you get started crafting your own custom configuration. 
 
 #### Basic Authentication