Skip to content

Commit b58a496

Browse files
Merge pull request #851 from fmount/pwd_validation
Move to VerifySecretFields to reject invalid passwords
2 parents 150119c + b44b23c commit b58a496

7 files changed

Lines changed: 84 additions & 63 deletions

File tree

internal/cloudkitty/common.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,14 @@ func EnsureTopology(
9191
func VerifyServiceSecret(
9292
ctx context.Context,
9393
secretName types.NamespacedName,
94-
expectedFields []string,
94+
expectedFields map[string]secret.Validator,
9595
reader client.Reader,
9696
conditionUpdater conditionUpdater,
9797
requeueTimeout time.Duration,
9898
envVars *map[string]env.Setter,
9999
) (ctrl.Result, error) {
100100

101-
hash, res, err := secret.VerifySecret(ctx, secretName, expectedFields, reader, requeueTimeout)
101+
hash, res, err := secret.VerifySecretFields(ctx, secretName, expectedFields, reader, requeueTimeout)
102102
if err != nil {
103103
conditionUpdater.Set(condition.FalseCondition(
104104
condition.InputReadyCondition,

internal/controller/autoscaling_controller.go

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,24 @@ func (r *AutoscalingReconciler) reconcileNormal(
370370
//
371371
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
372372
//
373-
ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Aodh.Secret, instance.Spec.Aodh.PasswordSelectors.AodhService, &configMapVars)
373+
// Associate to PasswordSelectors.Service field a password validator to
374+
// ensure pwd invalid detected patterns are rejected.
375+
validateFields := map[string]secret.Validator{
376+
instance.Spec.Aodh.PasswordSelectors.AodhService: secret.PasswordValidator{},
377+
}
378+
379+
_, ctrlResult, err := ensureSecret(
380+
ctx,
381+
types.NamespacedName{
382+
Namespace: instance.Namespace,
383+
Name: instance.Spec.Aodh.Secret,
384+
},
385+
validateFields,
386+
helper.GetClient(),
387+
&instance.Status.Conditions,
388+
&configMapVars,
389+
time.Duration(10)*time.Second,
390+
)
374391
if err != nil {
375392
return ctrlResult, err
376393
}
@@ -474,10 +491,29 @@ func (r *AutoscalingReconciler) reconcileNormal(
474491
return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
475492
}
476493

477-
ctrlResult, err = r.getSecret(ctx, helper, instance, *instance.Status.NotificationsURLSecret, "transport_url", &configMapVars)
494+
// transportURLFields are not pure password fields. We do not associate a
495+
// password validator and we only verify that the entry exists in the
496+
// secret
497+
transportValidateFields := map[string]secret.Validator{
498+
"transport_url": secret.NoOpValidator{},
499+
}
500+
501+
_, ctrlResult, err = ensureSecret(
502+
ctx,
503+
types.NamespacedName{
504+
Namespace: instance.Namespace,
505+
Name: *instance.Status.NotificationsURLSecret,
506+
},
507+
transportValidateFields,
508+
helper.GetClient(),
509+
&instance.Status.Conditions,
510+
&configMapVars,
511+
time.Duration(10)*time.Second,
512+
)
478513
if err != nil {
479514
return ctrlResult, err
480515
}
516+
481517
// run check TransportURL secret - end
482518

483519
//
@@ -875,29 +911,6 @@ func (r *AutoscalingReconciler) getAutoscalingHeat(
875911
return heat, err
876912
}
877913

878-
// getSecret - get the specified secret, and add its hash to envVars
879-
func (r *AutoscalingReconciler) getSecret(ctx context.Context, h *helper.Helper, instance *telemetryv1.Autoscaling, secretName string, expectedField string, envVars *map[string]env.Setter) (ctrl.Result, error) {
880-
secretHash, result, err := ensureSecret(
881-
ctx,
882-
types.NamespacedName{Namespace: instance.Namespace, Name: secretName},
883-
[]string{
884-
expectedField,
885-
},
886-
h.GetClient(),
887-
&instance.Status.Conditions,
888-
time.Duration(10)*time.Second,
889-
)
890-
if err != nil {
891-
return result, err
892-
}
893-
894-
// Add a prefix to the var name to avoid accidental collision with other non-secret
895-
// vars. The secret names themselves will be unique.
896-
(*envVars)["secret-"+secretName] = env.SetValue(secretHash)
897-
898-
return ctrl.Result{}, nil
899-
}
900-
901914
func (r *AutoscalingReconciler) transportURLCreateOrUpdate(
902915
ctx context.Context,
903916
instance *telemetryv1.Autoscaling,

internal/controller/ceilometer_controller.go

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,28 @@ func (r *CeilometerReconciler) reconcileCeilometer(
571571
//
572572
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
573573
//
574-
ctrlResult, err := r.getSecret(ctx, helper, instance, instance.Spec.Secret, instance.Spec.PasswordSelectors.CeilometerService, &configMapVars)
574+
// Associate to PasswordSelectors.Service field a password validator to
575+
// ensure pwd invalid detected patterns are rejected.
576+
validateFields := map[string]secret.Validator{
577+
instance.Spec.PasswordSelectors.CeilometerService: secret.PasswordValidator{},
578+
}
579+
580+
_, ctrlResult, err := ensureSecret(
581+
ctx,
582+
types.NamespacedName{
583+
Namespace: instance.Namespace,
584+
Name: instance.Spec.Secret,
585+
},
586+
validateFields,
587+
helper.GetClient(),
588+
&instance.Status.Conditions,
589+
&configMapVars,
590+
time.Duration(10)*time.Second,
591+
)
575592
if err != nil {
576593
return ctrlResult, err
577594
}
595+
578596
// run check OpenStack secret - end
579597

580598
//
@@ -1207,29 +1225,6 @@ func (r *CeilometerReconciler) reconcileKSM(
12071225
return ctrl.Result{}, nil
12081226
}
12091227

1210-
// getSecret - get the specified secret, and add its hash to envVars
1211-
func (r *CeilometerReconciler) getSecret(ctx context.Context, h *helper.Helper, instance *telemetryv1.Ceilometer, secretName string, expectedField string, envVars *map[string]env.Setter) (ctrl.Result, error) {
1212-
secretHash, result, err := ensureSecret(
1213-
ctx,
1214-
types.NamespacedName{Namespace: instance.Namespace, Name: secretName},
1215-
[]string{
1216-
expectedField,
1217-
},
1218-
h.GetClient(),
1219-
&instance.Status.Conditions,
1220-
time.Duration(10)*time.Second,
1221-
)
1222-
if err != nil {
1223-
return result, err
1224-
}
1225-
1226-
// Add a prefix to the var name to avoid accidental collision with other non-secret
1227-
// vars. The secret names themselves will be unique.
1228-
(*envVars)["secret-"+secretName] = env.SetValue(secretHash)
1229-
1230-
return ctrl.Result{}, nil
1231-
}
1232-
12331228
func (r *CeilometerReconciler) generateServiceConfig(
12341229
ctx context.Context,
12351230
h *helper.Helper,

internal/controller/cloudkitty_controller.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,12 +959,15 @@ func (r *CloudKittyReconciler) reconcileNormal(ctx context.Context, instance *te
959959
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
960960
//
961961

962+
// Associate to PasswordSelectors.Service field a password validator to
963+
// ensure pwd invalid detected patterns are rejected.
964+
validateFields := map[string]secret.Validator{
965+
instance.Spec.PasswordSelectors.CloudKittyService: secret.PasswordValidator{},
966+
}
962967
result, err := cloudkitty.VerifyServiceSecret(
963968
ctx,
964969
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
965-
[]string{
966-
instance.Spec.PasswordSelectors.CloudKittyService,
967-
},
970+
validateFields,
968971
helper.GetClient(),
969972
&instance.Status.Conditions,
970973
cloudkitty.NormalDuration,

internal/controller/cloudkittyapi_controller.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -767,12 +767,15 @@ func (r *CloudKittyAPIReconciler) reconcileNormal(ctx context.Context, instance
767767
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
768768
//
769769

770+
// Associate to PasswordSelectors.Service field a password validator to
771+
// ensure pwd invalid detected patterns are rejected.
772+
validateFields := map[string]secret.Validator{
773+
instance.Spec.PasswordSelectors.CloudKittyService: secret.PasswordValidator{},
774+
}
770775
ctrlResult, err := cloudkitty.VerifyServiceSecret(
771776
ctx,
772777
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
773-
[]string{
774-
instance.Spec.PasswordSelectors.CloudKittyService,
775-
},
778+
validateFields,
776779
helper.GetClient(),
777780
&instance.Status.Conditions,
778781
cloudkitty.NormalDuration,

internal/controller/cloudkittyproc_controller.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,13 +454,15 @@ func (r *CloudKittyProcReconciler) reconcileNormal(ctx context.Context, instance
454454
//
455455
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
456456
//
457-
457+
// Associate to PasswordSelectors.Service field a password validator to
458+
// ensure pwd invalid detected patterns are rejected.
459+
validateFields := map[string]secret.Validator{
460+
instance.Spec.PasswordSelectors.CloudKittyService: secret.PasswordValidator{},
461+
}
458462
ctrlResult, err := cloudkitty.VerifyServiceSecret(
459463
ctx,
460464
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
461-
[]string{
462-
instance.Spec.PasswordSelectors.CloudKittyService,
463-
},
465+
validateFields,
464466
helper.GetClient(),
465467
&instance.Status.Conditions,
466468
cloudkitty.NormalDuration,

internal/controller/telemetry_common.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
2626
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
27+
env "github.com/openstack-k8s-operators/lib-common/modules/common/env"
2728
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
2829
secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret"
2930
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -109,13 +110,14 @@ func ensureTopology(
109110
func ensureSecret(
110111
ctx context.Context,
111112
secretName types.NamespacedName,
112-
expectedFields []string,
113+
expectedFields map[string]secret.Validator,
113114
reader client.Reader,
114115
conditionUpdater conditionUpdater,
116+
envVars *map[string]env.Setter,
115117
requeueTimeout time.Duration,
116118
) (string, ctrl.Result, error) {
117119

118-
hash, res, err := secret.VerifySecret(ctx, secretName, expectedFields, reader, requeueTimeout)
120+
hash, res, err := secret.VerifySecretFields(ctx, secretName, expectedFields, reader, requeueTimeout)
119121
if err != nil {
120122
conditionUpdater.Set(condition.FalseCondition(
121123
condition.InputReadyCondition,
@@ -137,5 +139,8 @@ func ensureSecret(
137139
return "", res, nil
138140
}
139141

142+
// Add a prefix to the var name to avoid accidental collision with other non-secret
143+
// vars. The secret names themselves will be unique.
144+
(*envVars)["secret-"+secretName.Name] = env.SetValue(hash)
140145
return hash, ctrl.Result{}, nil
141146
}

0 commit comments

Comments
 (0)