You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/02/19 17:37:22 UTC
[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #221: Work with basic-auth enabled SolrCloud clusters
HoustonPutman commented on a change in pull request #221:
URL: https://github.com/apache/lucene-solr-operator/pull/221#discussion_r579331388
##########
File path: api/v1beta1/solrcloud_types.go
##########
@@ -1022,3 +1044,35 @@ type SolrTLSOptions struct {
// +optional
RestartOnTLSSecretUpdate bool `json:"restartOnTLSSecretUpdate,omitempty"`
}
+
+type SolrSecurityOptions struct {
+ // Secret containing credentials the operator should use for API requests to secure Solr pods.
+ // If you provide this secret, then the operator assumes you've also configured your own security.json file and
+ // uploaded it to Solr. The 'key' of the secret selector is the username. If you change the password for this
+ // user using the Solr security API, then you *must* update the secret with the new password or the operator will be
+ // locked out of Solr and API requests will fail, ultimately causing a CrashBackoffLoop for all pods if probe endpoints
+ // are secured.
+ //
+ // If you don't supply this secret, then the operator bootstraps a default security.json file and creates a
+ // corresponding secret containing the credentials for three users: admin, solr, and k8s-oper. All API requests
+ // from the operator are made as the 'k8s-oper' user, which is configured with minimal access. The 'solr' user has
+ // basic read access to Solr resources. Once the security.json is bootstrapped, the operator will not update it!
+ // You're expected to use the 'admin' user to access the Security API to make further changes. It's strictly a
+ // bootstrapping operation.
+ // +optional
+ BasicAuthSecret *corev1.SecretKeySelector `json:"basicAuthSecret,omitempty"`
+
+ // Flag to indicate if the configured HTTP endpoint(s) used for the probes require authentication; defaults
+ // to false. If you set to true, then probes will use a local command on the main container to hit the secured
+ // endpoints with credentials sourced from an env var instead of HTTP directly.
+ // +optional
+ ProbesRequireAuth bool `json:"probesRequireAuth,omitempty"`
+
+ // A list of endpoints that allow un-authenticated (aka "anonymous") access; this allows you to open Solr for
+ // un-authenticated access to query endpoints but lock down all other requests. This setting only applies during
+ // initial bootstrapping of the security.json file. Changing this after security.json has been applied by the
+ // operator to a SolrCloud instance has no effect. Obviously, if you're supplying your own basicAuthSecret, then this
+ // setting does not apply as you're expected to configure your own security.json.
+ // +optional
+ InitAnonymousEndpoints []string `json:"initAnonymousEndpoints,omitempty"`
Review comment:
After sleeping on it, I'm of the opinion that we should axe this. I think it crosses the boundary between "what should be managed by Solr" and "What should be managed by the operator".
We can link to the solr documentation for the feature, allowing users to make this change after the cluster is live.
##########
File path: controllers/solrcloud_controller.go
##########
@@ -256,6 +257,55 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
+ basicAuthCreds := ""
+ if instance.Spec.SolrSecurity != nil {
+ ctx := context.TODO()
+ sec := instance.Spec.SolrSecurity
+ username := solr.DefaultBasicAuthUsername
+ secretName := instance.BasicAuthSecretName()
+ basicAuthSecret := &corev1.Secret{}
+
+ // user has the option of providing a secret with credentials the operator should use to make requests to Solr
+ if sec.BasicAuthSecret != nil {
+ if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, basicAuthSecret); err != nil {
+ return requeueOrNot, err
+ }
+
+ if _, ok := basicAuthSecret.Data[sec.BasicAuthSecret.Key]; !ok {
+ return requeueOrNot, fmt.Errorf("required %s key not found in user-provided basic-auth secret %s",
+ sec.BasicAuthSecret.Key, secretName)
+ }
+
+ // the username is the key from the user-provided secret
+ username = sec.BasicAuthSecret.Key
+ } else {
+ // We're supplying a secret with random passwords and a default security.json
+ // since we randomly generate the passwords, we need to lookup the secret first and only create if not exist
+ err = r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, basicAuthSecret)
+ if err != nil && errors.IsNotFound(err) {
+ newSecret := util.GenerateBasicAuthSecret(instance)
+ if err := controllerutil.SetControllerReference(instance, newSecret, r.scheme); err != nil {
+ return requeueOrNot, err
+ }
+ err = r.Create(ctx, newSecret)
+ basicAuthSecret = newSecret
+ }
+ if err != nil {
+ return requeueOrNot, err
+ }
+ // supply the bootstrap security.json to the initContainer via a simple BASE64 encoding env var
+ configMapInfo[util.SecurityJsonFile] = string(basicAuthSecret.Data[util.SecurityJsonFile])
+ }
+
+ // save some basic information needed to configure probes for auth-enabled Solr pods
+ configMapInfo[util.SecuritySecret] = basicAuthSecret.Name
Review comment:
shouldn't you only set this if probes should use the auth data?
Also maybe we should rename this from configMapInfo to something else...
##########
File path: controllers/util/solr_util.go
##########
@@ -1158,3 +1195,200 @@ func createZkConnectionEnvVars(solrCloud *solr.SolrCloud, solrCloudStatus *solr.
return envVars, solrOpt, len(zkChroot) > 1
}
+
+func setupVolumeMountForUserProvidedConfigMapEntry(configMapInfo map[string]string, fileKey string, solrVolumes []corev1.Volume, envVar string) (*corev1.VolumeMount, *corev1.EnvVar, *corev1.Volume) {
+ volName := strings.ReplaceAll(fileKey, ".", "-")
+ mountPath := fmt.Sprintf("/var/solr/%s", configMapInfo[fileKey])
+ appendedToExisting := false
+ if configMapInfo[fileKey] == configMapInfo[SolrXmlFile] {
+ // the user provided a custom log4j2.xml and solr.xml, append to the volume for solr.xml created above
+ for _, vol := range solrVolumes {
+ if vol.Name == "solr-xml" {
+ vol.ConfigMap.Items = append(vol.ConfigMap.Items, corev1.KeyToPath{Key: fileKey, Path: fileKey})
+ appendedToExisting = true
+ volName = vol.Name
+ break
+ }
+ }
+ }
+
+ var vol *corev1.Volume = nil
+ if !appendedToExisting {
+ defaultMode := int32(420)
+ vol = &corev1.Volume{
+ Name: volName,
+ VolumeSource: corev1.VolumeSource{
+ ConfigMap: &corev1.ConfigMapVolumeSource{
+ LocalObjectReference: corev1.LocalObjectReference{Name: configMapInfo[fileKey]},
+ Items: []corev1.KeyToPath{{Key: fileKey, Path: fileKey}},
+ DefaultMode: &defaultMode,
+ },
+ },
+ }
+ }
+ pathToFile := fmt.Sprintf("%s/%s", mountPath, fileKey)
+
+ return &corev1.VolumeMount{Name: volName, MountPath: mountPath}, &corev1.EnvVar{Name: envVar, Value: pathToFile}, vol
+}
+
+func GenerateBasicAuthSecret(solrCloud *solr.SolrCloud) *corev1.Secret {
+ labels := solrCloud.SharedLabelsWith(solrCloud.GetLabels())
+ var annotations map[string]string
+ return &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: solrCloud.BasicAuthSecretName(),
+ Namespace: solrCloud.GetNamespace(),
+ Labels: labels,
+ Annotations: annotations,
+ },
+ Data: generateSecurityJson(solrCloud),
+ Type: "Opaque",
Review comment:
We might want to use `Type: "kubernetes.io/basic-auth"`?
##########
File path: controllers/solrcloud_controller.go
##########
@@ -256,6 +257,55 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
+ basicAuthCreds := ""
+ if instance.Spec.SolrSecurity != nil {
+ ctx := context.TODO()
+ sec := instance.Spec.SolrSecurity
+ username := solr.DefaultBasicAuthUsername
+ secretName := instance.BasicAuthSecretName()
+ basicAuthSecret := &corev1.Secret{}
+
+ // user has the option of providing a secret with credentials the operator should use to make requests to Solr
+ if sec.BasicAuthSecret != nil {
+ if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, basicAuthSecret); err != nil {
+ return requeueOrNot, err
+ }
+
+ if _, ok := basicAuthSecret.Data[sec.BasicAuthSecret.Key]; !ok {
+ return requeueOrNot, fmt.Errorf("required %s key not found in user-provided basic-auth secret %s",
+ sec.BasicAuthSecret.Key, secretName)
+ }
+
+ // the username is the key from the user-provided secret
+ username = sec.BasicAuthSecret.Key
+ } else {
+ // We're supplying a secret with random passwords and a default security.json
+ // since we randomly generate the passwords, we need to lookup the secret first and only create if not exist
+ err = r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, basicAuthSecret)
+ if err != nil && errors.IsNotFound(err) {
+ newSecret := util.GenerateBasicAuthSecret(instance)
+ if err := controllerutil.SetControllerReference(instance, newSecret, r.scheme); err != nil {
+ return requeueOrNot, err
+ }
+ err = r.Create(ctx, newSecret)
+ basicAuthSecret = newSecret
+ }
+ if err != nil {
+ return requeueOrNot, err
+ }
+ // supply the bootstrap security.json to the initContainer via a simple BASE64 encoding env var
+ configMapInfo[util.SecurityJsonFile] = string(basicAuthSecret.Data[util.SecurityJsonFile])
+ }
+
+ // save some basic information needed to configure probes for auth-enabled Solr pods
+ configMapInfo[util.SecuritySecret] = basicAuthSecret.Name
+ configMapInfo[util.SecurityUsername] = username
+
+ // need the creds below for getting CLUSTERSTATUS
+ creds := username + ":" + string(basicAuthSecret.Data[username])
Review comment:
does the Data need to be decoded from base64?
##########
File path: controllers/solrprometheusexporter_controller.go
##########
@@ -286,6 +301,13 @@ func (r *SolrPrometheusExporterReconciler) SetupWithManagerAndReconciler(mgr ctr
return err
}
+ // Get notified when the basic auth secret updates; exporter pods must be restarted if the basic auth password
+ // changes b/c the credentials are loaded from a Java system property at startup and not watched for changes.
+ ctrlBuilder, err = r.indexAndWatchForBasicAuthSecret(mgr, ctrlBuilder)
Review comment:
Honestly think at this point we should bake something into ControllerRuntime to automate this. We use it so often.
(Nothing to do with this PR, just an observation)
##########
File path: api/v1beta1/solrcloud_types.go
##########
@@ -1022,3 +1044,35 @@ type SolrTLSOptions struct {
// +optional
RestartOnTLSSecretUpdate bool `json:"restartOnTLSSecretUpdate,omitempty"`
}
+
+type SolrSecurityOptions struct {
+ // Secret containing credentials the operator should use for API requests to secure Solr pods.
+ // If you provide this secret, then the operator assumes you've also configured your own security.json file and
+ // uploaded it to Solr. The 'key' of the secret selector is the username. If you change the password for this
+ // user using the Solr security API, then you *must* update the secret with the new password or the operator will be
+ // locked out of Solr and API requests will fail, ultimately causing a CrashBackoffLoop for all pods if probe endpoints
+ // are secured.
+ //
+ // If you don't supply this secret, then the operator bootstraps a default security.json file and creates a
+ // corresponding secret containing the credentials for three users: admin, solr, and k8s-oper. All API requests
+ // from the operator are made as the 'k8s-oper' user, which is configured with minimal access. The 'solr' user has
+ // basic read access to Solr resources. Once the security.json is bootstrapped, the operator will not update it!
+ // You're expected to use the 'admin' user to access the Security API to make further changes. It's strictly a
+ // bootstrapping operation.
+ // +optional
+ BasicAuthSecret *corev1.SecretKeySelector `json:"basicAuthSecret,omitempty"`
Review comment:
I am not familiar with how other CRDs handle this, but should the "key" have to be the username, or should it be passed in separately?
Like `basicAuthUsername` and `basicAuthPasswordSecret`.
[Looking at the kubernetes docs](https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret), it looks like the standard for Basic Auth secrets is to store "username": <> and "password": <>.
##########
File path: controllers/solrcloud_controller.go
##########
@@ -256,6 +257,55 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}
+ basicAuthCreds := ""
+ if instance.Spec.SolrSecurity != nil {
+ ctx := context.TODO()
+ sec := instance.Spec.SolrSecurity
+ username := solr.DefaultBasicAuthUsername
+ secretName := instance.BasicAuthSecretName()
+ basicAuthSecret := &corev1.Secret{}
+
+ // user has the option of providing a secret with credentials the operator should use to make requests to Solr
+ if sec.BasicAuthSecret != nil {
+ if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, basicAuthSecret); err != nil {
+ return requeueOrNot, err
+ }
+
+ if _, ok := basicAuthSecret.Data[sec.BasicAuthSecret.Key]; !ok {
Review comment:
Could be in Data or StringData
##########
File path: api/v1beta1/solrprometheusexporter_types.go
##########
@@ -88,6 +89,13 @@ type SolrReference struct {
// Settings to configure the SolrJ client used to request metrics from TLS enabled Solr pods
// +optional
SolrTLS *SolrTLSOptions `json:"solrTLS,omitempty"`
+
+ // If Solr is secured, you'll need to provide credentials for the Prometheus exporter to authenticate.
+ // The key is the username. If basic auth is enabled on the SolrCloud instance, the default secret (unless you are
+ // supplying your own) is named using the pattern: <SOLR_CLOUD_NAME>-solrcloud-basic-auth. If using the security.json
+ // bootstrapped by the Solr operator, then the username should be "k8s-oper".
+ // +optional
+ BasicAuthSecret *corev1.SecretKeySelector `json:"basicAuthSecret,omitempty"`
Review comment:
Same comment about basicAuth secrets standards.
##########
File path: controllers/solrprometheusexporter_controller.go
##########
@@ -209,7 +209,22 @@ func (r *SolrPrometheusExporterReconciler) Reconcile(req ctrl.Request) (ctrl.Res
}
}
- deploy := util.GenerateSolrPrometheusExporterDeployment(prometheusExporter, solrConnectionInfo, configXmlMd5, tlsClientOptions)
+ basicAuthMd5 := ""
+ if prometheusExporter.Spec.SolrReference.BasicAuthSecret != nil {
+ selector := prometheusExporter.Spec.SolrReference.BasicAuthSecret
+ basicAuthSecret := &corev1.Secret{}
+ err := r.Get(context.TODO(), types.NamespacedName{Name: selector.Name, Namespace: prometheusExporter.Namespace}, basicAuthSecret)
+ if err != nil {
+ return reconcile.Result{}, err
+ }
+ passBytes, ok := basicAuthSecret.Data[selector.Key]
Review comment:
We might have to look in two places here. It could be in `data` or `stringData`.
https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org