Skip to content

Commit 9f43f52

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #683 from fmount/pwd_validation
Move to VerifySecretFields to reject invalid passwords
2 parents 4bd0f16 + d1191ad commit 9f43f52

File tree

7 files changed

+313
-145
lines changed

7 files changed

+313
-145
lines changed

‎internal/controller/ironic_controller.go‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -376,28 +376,41 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic
376376
//
377377
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
378378
//
379-
ospSecret, hash, err := oko_secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
379+
// Associate to PasswordSelectors.Service field a password validator to
380+
// ensure pwd invalid detected patterns are rejected.
381+
validateFields := map[string]oko_secret.Validator{
382+
instance.Spec.PasswordSelectors.Service: oko_secret.PasswordValidator{},
383+
}
384+
hash, ctrlResult, err := oko_secret.VerifySecretFields(
385+
ctx,
386+
types.NamespacedName{
387+
Namespace: instance.Namespace,
388+
Name: instance.Spec.Secret,
389+
},
390+
validateFields,
391+
helper.GetClient(),
392+
time.Duration(10)*time.Second,
393+
)
380394
if err != nil {
381-
if k8s_errors.IsNotFound(err) {
382-
// Since the OpenStack secret should have been manually created by the user and referenced in the spec,
383-
// we treat this as a warning because it means that the service will not be able to start.
384-
Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
385-
instance.Status.Conditions.Set(condition.FalseCondition(
386-
condition.InputReadyCondition,
387-
condition.ErrorReason,
388-
condition.SeverityWarning,
389-
condition.InputReadyWaitingMessage))
390-
return ctrl.Result{}, nil
391-
}
392395
instance.Status.Conditions.Set(condition.FalseCondition(
393396
condition.InputReadyCondition,
394397
condition.ErrorReason,
395398
condition.SeverityWarning,
396399
condition.InputReadyErrorMessage,
397400
err.Error()))
398-
return ctrl.Result{}, err
401+
return ctrlResult, err
402+
} else if (ctrlResult != ctrl.Result{}) {
403+
// Since the service secret should have been manually created by the user and referenced in the spec,
404+
// we treat this as a warning because it means that the service will not be able to start.
405+
log.FromContext(ctx).Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
406+
instance.Status.Conditions.Set(condition.FalseCondition(
407+
condition.InputReadyCondition,
408+
condition.ErrorReason,
409+
condition.SeverityWarning,
410+
condition.InputReadyWaitingMessage))
411+
return ctrlResult, err
399412
}
400-
configMapVars[ospSecret.Name] = env.SetValue(hash)
413+
configMapVars[instance.Spec.Secret] = env.SetValue(hash)
401414

402415
instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
403416
// run check OpenStack secret - end
@@ -480,7 +493,7 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic
480493
}
481494

482495
// Handle service upgrade
483-
ctrlResult, err := r.reconcileUpgrade(ctx, instance, helper, serviceLabels)
496+
ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper, serviceLabels)
484497
if err != nil {
485498
return ctrlResult, err
486499
} else if (ctrlResult != ctrl.Result{}) {

‎internal/controller/ironicapi_controller.go‎

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -676,57 +676,81 @@ func (r *IronicAPIReconciler) reconcileNormal(ctx context.Context, instance *iro
676676
//
677677
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
678678
//
679-
ospSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
679+
// Associate to PasswordSelectors.Service field a password validator to
680+
// ensure pwd invalid detected patterns are rejected.
681+
validateFields := map[string]secret.Validator{
682+
instance.Spec.PasswordSelectors.Service: secret.PasswordValidator{},
683+
}
684+
hash, ctrlResult, err := secret.VerifySecretFields(
685+
ctx,
686+
types.NamespacedName{
687+
Namespace: instance.Namespace,
688+
Name: instance.Spec.Secret,
689+
},
690+
validateFields,
691+
helper.GetClient(),
692+
time.Duration(10)*time.Second,
693+
)
680694
if err != nil {
681-
if k8s_errors.IsNotFound(err) {
682-
// Since the OpenStack secret should have been manually created by the user and referenced in the spec,
683-
// we treat this as a warning because it means that the service will not be able to start.
684-
Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
685-
instance.Status.Conditions.Set(condition.FalseCondition(
686-
condition.InputReadyCondition,
687-
condition.ErrorReason,
688-
condition.SeverityWarning,
689-
condition.InputReadyWaitingMessage))
690-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
691-
}
692695
instance.Status.Conditions.Set(condition.FalseCondition(
693696
condition.InputReadyCondition,
694697
condition.ErrorReason,
695698
condition.SeverityWarning,
696699
condition.InputReadyErrorMessage,
697700
err.Error()))
698-
return ctrl.Result{}, err
701+
return ctrlResult, err
702+
} else if (ctrlResult != ctrl.Result{}) {
703+
// Since the service secret should have been manually created by the user and referenced in the spec,
704+
// we treat this as a warning because it means that the service will not be able to start.
705+
log.FromContext(ctx).Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
706+
instance.Status.Conditions.Set(condition.FalseCondition(
707+
condition.InputReadyCondition,
708+
condition.ErrorReason,
709+
condition.SeverityWarning,
710+
condition.InputReadyWaitingMessage))
711+
return ctrlResult, err
699712
}
700-
configMapVars[ospSecret.Name] = env.SetValue(hash)
713+
configMapVars[instance.Spec.Secret] = env.SetValue(hash)
701714
// run check OpenStack secret - end
702715

703716
//
704717
// check for required TransportURL secret holding transport URL string
705718
//
706719
if instance.Spec.RPCTransport == "oslo" {
707-
transportURLSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.TransportURLSecret, instance.Namespace)
720+
// transportURLFields are not pure password fields. We do not associate a
721+
// password validator and we only verify that the entry exists in the
722+
// secret
723+
transportValidateFields := map[string]secret.Validator{
724+
"transport_url": secret.NoOpValidator{},
725+
}
726+
hash, ctrlResult, err = secret.VerifySecretFields(
727+
ctx,
728+
types.NamespacedName{
729+
Namespace: instance.Namespace,
730+
Name: instance.Spec.TransportURLSecret,
731+
},
732+
transportValidateFields,
733+
helper.GetClient(),
734+
time.Duration(10)*time.Second,
735+
)
708736
if err != nil {
709-
if k8s_errors.IsNotFound(err) {
710-
// Since the TransportURL secret should have been previously automatically created by the parent
711-
// Ironic CR and then referenced in this instance's spec, we treat this as a warning because it
712-
// means that the service will not be able to start.
713-
Log.Info(fmt.Sprintf("TransportURL secret %s not found", instance.Spec.TransportURLSecret))
714-
instance.Status.Conditions.Set(condition.FalseCondition(
715-
condition.InputReadyCondition,
716-
condition.ErrorReason,
717-
condition.SeverityWarning,
718-
condition.InputReadyWaitingMessage))
719-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
720-
}
721737
instance.Status.Conditions.Set(condition.FalseCondition(
722738
condition.InputReadyCondition,
723739
condition.ErrorReason,
724740
condition.SeverityWarning,
725741
condition.InputReadyErrorMessage,
726742
err.Error()))
727-
return ctrl.Result{}, err
743+
return ctrlResult, err
744+
} else if (ctrlResult != ctrl.Result{}) {
745+
Log.Info(fmt.Sprintf("TransportURL secret %s not found", instance.Spec.TransportURLSecret))
746+
instance.Status.Conditions.Set(condition.FalseCondition(
747+
condition.InputReadyCondition,
748+
condition.RequestedReason,
749+
condition.SeverityInfo,
750+
condition.InputReadyWaitingMessage))
751+
return ctrlResult, err
728752
}
729-
configMapVars[transportURLSecret.Name] = env.SetValue(hash)
753+
configMapVars[instance.Spec.TransportURLSecret] = env.SetValue(hash)
730754
}
731755
// run check TransportURL secret - end
732756

@@ -877,7 +901,7 @@ func (r *IronicAPIReconciler) reconcileNormal(ctx context.Context, instance *iro
877901
}
878902

879903
// Handle service init
880-
ctrlResult, err := r.reconcileInit(ctx, instance, helper, serviceLabels)
904+
ctrlResult, err = r.reconcileInit(ctx, instance, helper, serviceLabels)
881905
if err != nil {
882906
return ctrlResult, err
883907
} else if (ctrlResult != ctrl.Result{}) {

‎internal/controller/ironicconductor_controller.go‎

Lines changed: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -550,57 +550,82 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc
550550
//
551551
// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
552552
//
553-
ospSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
553+
// Associate to PasswordSelectors.Service field a password validator to
554+
// ensure pwd invalid detected patterns are rejected.
555+
validateFields := map[string]secret.Validator{
556+
instance.Spec.PasswordSelectors.Service: secret.PasswordValidator{},
557+
}
558+
hash, ctrlResult, err := secret.VerifySecretFields(
559+
ctx,
560+
types.NamespacedName{
561+
Namespace: instance.Namespace,
562+
Name: instance.Spec.Secret,
563+
},
564+
validateFields,
565+
helper.GetClient(),
566+
time.Duration(10)*time.Second,
567+
)
554568
if err != nil {
555-
if k8s_errors.IsNotFound(err) {
556-
// Since the OpenStack secret should have been manually created by the user and referenced in the spec,
557-
// we treat this as a warning because it means that the service will not be able to start.
558-
Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
559-
instance.Status.Conditions.Set(condition.FalseCondition(
560-
condition.InputReadyCondition,
561-
condition.ErrorReason,
562-
condition.SeverityWarning,
563-
condition.InputReadyWaitingMessage))
564-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
565-
}
566569
instance.Status.Conditions.Set(condition.FalseCondition(
567570
condition.InputReadyCondition,
568571
condition.ErrorReason,
569572
condition.SeverityWarning,
570573
condition.InputReadyErrorMessage,
571574
err.Error()))
572-
return ctrl.Result{}, err
575+
return ctrlResult, err
576+
} else if (ctrlResult != ctrl.Result{}) {
577+
// Since the service secret should have been manually created by the user and referenced in the spec,
578+
// we treat this as a warning because it means that the service will not be able to start.
579+
log.FromContext(ctx).Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret))
580+
instance.Status.Conditions.Set(condition.FalseCondition(
581+
condition.InputReadyCondition,
582+
condition.ErrorReason,
583+
condition.SeverityWarning,
584+
condition.InputReadyWaitingMessage))
585+
return ctrlResult, err
573586
}
574-
configMapVars[ospSecret.Name] = env.SetValue(hash)
587+
configMapVars[instance.Spec.Secret] = env.SetValue(hash)
575588
// run check OpenStack secret - end
576589

577590
//
578591
// check for required TransportURL secret holding transport URL string
579592
//
580593
if instance.Spec.RPCTransport == "oslo" {
581-
transportURLSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.TransportURLSecret, instance.Namespace)
594+
// transportURLFields are not pure password fields. We do not associate a
595+
// password validator and we only verify that the entry exists in the
596+
// secret
597+
transportValidateFields := map[string]secret.Validator{
598+
"transport_url": secret.NoOpValidator{},
599+
}
600+
hash, ctrlResult, err = secret.VerifySecretFields(
601+
ctx,
602+
types.NamespacedName{
603+
Namespace: instance.Namespace,
604+
Name: instance.Spec.TransportURLSecret,
605+
},
606+
transportValidateFields,
607+
helper.GetClient(),
608+
time.Duration(10)*time.Second,
609+
)
610+
// NOTE: This should be moved to TransportURLSecretReadyCondition
582611
if err != nil {
583-
if k8s_errors.IsNotFound(err) {
584-
// Since the TransportURL secret should have been previously automatically created by the parent
585-
// Ironic CR and then referenced in this instance's spec, we treat this as a warning because it
586-
// means that the service will not be able to start.
587-
Log.Info(fmt.Sprintf("TransportURL secret %s not found", instance.Spec.TransportURLSecret))
588-
instance.Status.Conditions.Set(condition.FalseCondition(
589-
condition.InputReadyCondition,
590-
condition.ErrorReason,
591-
condition.SeverityWarning,
592-
condition.InputReadyWaitingMessage))
593-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
594-
}
595612
instance.Status.Conditions.Set(condition.FalseCondition(
596613
condition.InputReadyCondition,
597614
condition.ErrorReason,
598615
condition.SeverityWarning,
599616
condition.InputReadyErrorMessage,
600617
err.Error()))
601-
return ctrl.Result{}, err
618+
return ctrlResult, err
619+
} else if (ctrlResult != ctrl.Result{}) {
620+
Log.Info(fmt.Sprintf("TransportURL secret %s not found", instance.Spec.TransportURLSecret))
621+
instance.Status.Conditions.Set(condition.FalseCondition(
622+
condition.InputReadyCondition,
623+
condition.RequestedReason,
624+
condition.SeverityInfo,
625+
condition.InputReadyWaitingMessage))
626+
return ctrlResult, err
602627
}
603-
configMapVars[transportURLSecret.Name] = env.SetValue(hash)
628+
configMapVars[instance.Spec.TransportURLSecret] = env.SetValue(hash)
604629
}
605630
// run check TransportURL secret - end
606631

@@ -772,7 +797,7 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc
772797
time.Duration(5)*time.Second,
773798
)
774799

775-
ctrlResult, err := ss.CreateOrPatch(ctx, helper)
800+
ctrlResult, err = ss.CreateOrPatch(ctx, helper)
776801
if err != nil {
777802
instance.Status.Conditions.Set(condition.FalseCondition(
778803
condition.DeploymentReadyCondition,

0 commit comments

Comments
 (0)