THREESCALE-10518 - address review notes

3scale · Jan 14, 2024 · 59963bd · 59963bd
1 parent f007f1b
commit 59963bd
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 7 deletions.
diff --git a/controllers/capabilities/openapi_controller.go b/controllers/capabilities/openapi_controller.go
@@ -400,6 +400,16 @@ func (r *OpenAPIReconciler) validateOIDCSettingsInCR(openapiCR *capabilitiesv1be
 	specFldPath := field.NewPath("spec")
 	openapiRefFldPath := specFldPath.Child("openapiRef")
 
+	if openapiCR.Spec.OIDC != nil &&
+		(openapiCR.Spec.OIDC.IssuerEndpoint == "" && openapiCR.Spec.OIDC.IssuerEndpointRef == nil) {
+		fieldErrors = append(fieldErrors, field.Invalid(openapiRefFldPath, openapiCR.Spec.OpenAPIRef, "OIDC issuer endpoint definition is missing in CR - "+
+			"No IssuerEndpoint nor IssuerEndpointRef found in OIDC spec in CR, one of them must be set."))
+		return &helper.SpecFieldError{
+			ErrorType:      helper.InvalidError,
+			FieldErrorList: fieldErrors,
+		}
+	}
+
 	globalSecRequirements := helper.OpenAPIGlobalSecurityRequirements(openapiObj)
 	if len(globalSecRequirements) == 0 && openapiCR.Spec.OIDC != nil {
 		logger.Info("OIDC definitions in CR will be ignored, as no security requirements are found. Default to UserKey authentication")
@@ -420,11 +430,5 @@ func (r *OpenAPIReconciler) validateOIDCSettingsInCR(openapiCR *capabilitiesv1be
 		}
 	}
 
-	if openapiCR.Spec.OIDC != nil &&
-		(openapiCR.Spec.OIDC.IssuerEndpoint == "" && openapiCR.Spec.OIDC.IssuerEndpointRef == nil) {
-		logger.Info("OIDC issuer endpoint definition is missing, as no IssuerEndpoint nor IssuerEndpointRef found in CR.")
-		r.EventRecorder().Eventf(openapiCR, corev1.EventTypeWarning, "OIDC issuer endpoint definition is missing in CR", "%v", "No IssuerEndpoint nor IssuerEndpointRef found in OIDC spec in CR; please set it to fix the problem.")
-	}
-
 	return nil
 }
diff --git a/doc/openapi-user-guide.md b/doc/openapi-user-guide.md
@@ -151,7 +151,7 @@ spec:
 - Only for OIDC: 
 
 | **Field**                | **Required** | **Description**                                                                                                                                                                                                                                                                                                                                                                                                                                       |
-| --- | --- |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| --- | --- | --- |
 | issuerType               | no           | Valid values: [keycloak, rest]. Defaults to `rest`                                                                                                                                                                                                                                                                                                                                                                                                    |
 | issuerEndpoint           | no           | Issuer endpoint. It can be defined in `issuerEndpointRef` or as plain value (please see CR example and notes below). The format of this endpoint is determined on your OpenID Provider setup. For RHSSO:  https://<client_id>:<client_secret>@<host>:<port>/auth/realms/<realm_name>                                                                                                                                                                  |
 | issuerEndpointRef        | no           | The secret that contains  `issuerEndpoint`                                                                                                                                                                                                                                                                                                                                                                                                            |