Merge pull request #197 from Deydra71/httpd-config

Add configurable API Timeout
openstack-k8s-operators · Jan 3, 2025 · 02efac8 · 02efac8
2 parents 619609b + 9ad33de
commit 02efac8
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 6 deletions.
diff --git a/api/bases/barbican.openstack.org_barbicanapis.yaml b/api/bases/barbican.openstack.org_barbicanapis.yaml
@@ -48,6 +48,10 @@ spec:
           spec:
             description: BarbicanAPISpec defines the desired state of BarbicanAPI
             properties:
+              apiTimeout:
+                description: APITimeout for HAProxy and Apache defaults to Barbican
+                  APITimeout (seconds)
+                type: integer
               containerImage:
                 description: ContainerImage - Barbican Container Image URL (will be
                   set to environmental default if empty)

diff --git a/api/bases/barbican.openstack.org_barbicans.yaml b/api/bases/barbican.openstack.org_barbicans.yaml
@@ -48,10 +48,18 @@ spec:
           spec:
             description: BarbicanSpec defines the desired state of Barbican
             properties:
+              apiTimeout:
+                default: 90
+                description: Barbican API timeout
+                type: integer
               barbicanAPI:
                 description: BarbicanAPI - Spec definition for the  API services of
                   this Barbican deployment
                 properties:
+                  apiTimeout:
+                    description: APITimeout for HAProxy and Apache defaults to Barbican
+                      APITimeout (seconds)
+                    type: integer
                   containerImage:
                     description: ContainerImage - Barbican Container Image URL (will
                       be set to environmental default if empty)

diff --git a/api/v1beta1/barbican_types.go b/api/v1beta1/barbican_types.go
@@ -42,6 +42,9 @@ const (
 
 	// BarbicanKeystoneListenerContainerImage is the fall-back container image for BarbicanAPI
 	BarbicanKeystoneListenerContainerImage = "quay.io/podified-antelope-centos9/openstack-barbican-keystone-listener:current-podified"
+
+	// Barbican API timeout
+	APITimeout = 90
 )
 
 // BarbicanSpec defines the desired state of Barbican
@@ -106,6 +109,11 @@ type BarbicanSpecBase struct {
 
 	// +kubebuilder:validation:Required
 	// BarbicanAPIInternal - Spec definition for the internal and admin API service of this Barbican deployment
+
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=90
+	// Barbican API timeout
+	APITimeout int `json:"apiTimeout"`
 }
 
 // BarbicanStatus defines the observed state of Barbican
@@ -195,6 +203,7 @@ func SetupDefaults() {
 		APIContainerImageURL:              util.GetEnvVar("RELATED_IMAGE_BARBICAN_API_IMAGE_URL_DEFAULT", BarbicanAPIContainerImage),
 		WorkerContainerImageURL:           util.GetEnvVar("RELATED_IMAGE_BARBICAN_WORKER_IMAGE_URL_DEFAULT", BarbicanWorkerContainerImage),
 		KeystoneListenerContainerImageURL: util.GetEnvVar("RELATED_IMAGE_BARBICAN_KEYSTONE_LISTENER_IMAGE_URL_DEFAULT", BarbicanKeystoneListenerContainerImage),
+		BarbicanAPITimeout:                APITimeout,
 	}
 
 	SetupBarbicanDefaults(barbicanDefaults)

diff --git a/api/v1beta1/barbican_webhook.go b/api/v1beta1/barbican_webhook.go
@@ -42,6 +42,7 @@ type BarbicanDefaults struct {
 	APIContainerImageURL              string
 	WorkerContainerImageURL           string
 	KeystoneListenerContainerImageURL string
+	BarbicanAPITimeout                int
 }
 
 var barbicanDefaults BarbicanDefaults
@@ -135,21 +136,20 @@ func (r *BarbicanSpec) ValidateCreate(basePath *field.Path) field.ErrorList {
 		r.BarbicanAPI.Override.Service)...)
 
 	// pkcs11 verifications
-        if slices.Contains(r.EnabledSecretStores, "pkcs11") {
-                if r.PKCS11 == nil {
+	if slices.Contains(r.EnabledSecretStores, "pkcs11") {
+		if r.PKCS11 == nil {
 			allErrs = append(allErrs, field.Required(basePath.Child("PKCS11"),
 				"PKCS11 specification is missing, PKCS11 is required when pkcs11 is an enabled SecretStore"),
 			)
-                } else {
+		} else {
 			// Checking that at least one of the following parameters has been provided.
 			if len(r.PKCS11.TokenSerialNumber) == 0 && len(r.PKCS11.TokenLabels) == 0 && len(r.PKCS11.SlotId) == 0 {
 				allErrs = append(allErrs, field.Required(basePath.Child("PKCS11"),
 					"No token identifier provided. One of TokenSerialNumber, TokenLabels or SlotId needed"),
 				)
 			}
-                }
-        }
-
+		}
+	}
 
 	return allErrs
 }
@@ -221,3 +221,33 @@ func (r *Barbican) ValidateDelete() (admission.Warnings, error) {
 	// TODO(user): fill in your validation logic upon object deletion.
 	return nil, nil
 }
+
+func (spec *BarbicanSpecCore) GetDefaultRouteAnnotations() (annotations map[string]string) {
+	return map[string]string{
+		"haproxy.router.openshift.io/timeout": fmt.Sprintf("%ds", barbicanDefaults.BarbicanAPITimeout),
+	}
+}
+
+// SetDefaultRouteAnnotations sets HAProxy timeout values for Barbican API routes
+func (spec *BarbicanAPITemplateCore) SetDefaultRouteAnnotations(annotations map[string]string) {
+	const haProxyAnno = "haproxy.router.openshift.io/timeout"
+	// Use a custom annotation to flag when the operator has set the default HAProxy timeout
+	// With the annotation func determines when to overwrite existing HAProxy timeout with the APITimeout
+	const barbicanAnno = "api.Barbican.openstack.org/timeout"
+	valBarbicanAPI, okBarbicanAPI := annotations[barbicanAnno]
+	valHAProxy, okHAProxy := annotations[haProxyAnno]
+
+	// Human operator set the HAProxy timeout manually
+	if !okBarbicanAPI && okHAProxy {
+		return
+	}
+	// Human operator modified the HAProxy timeout manually without removing the Barbican flag
+	if okBarbicanAPI && okHAProxy && valBarbicanAPI != valHAProxy {
+		delete(annotations, barbicanAnno)
+		return
+	}
+
+	timeout := fmt.Sprintf("%ds", spec.APITimeout)
+	annotations[barbicanAnno] = timeout
+	annotations[haProxyAnno] = timeout
+}
diff --git a/api/v1beta1/barbicanapi_types.go b/api/v1beta1/barbicanapi_types.go
@@ -49,6 +49,10 @@ type BarbicanAPITemplateCore struct {
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// TLS - Parameters related to the TLS
 	TLS tls.API `json:"tls,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	// APITimeout for HAProxy and Apache defaults to Barbican APITimeout (seconds)
+	APITimeout int `json:"apiTimeout"`
 }
 
 // APIOverrideSpec to override the generated manifest of several child resources.

diff --git a/config/crd/bases/barbican.openstack.org_barbicanapis.yaml b/config/crd/bases/barbican.openstack.org_barbicanapis.yaml
@@ -48,6 +48,10 @@ spec:
           spec:
             description: BarbicanAPISpec defines the desired state of BarbicanAPI
             properties:
+              apiTimeout:
+                description: APITimeout for HAProxy and Apache defaults to Barbican
+                  APITimeout (seconds)
+                type: integer
               containerImage:
                 description: ContainerImage - Barbican Container Image URL (will be
                   set to environmental default if empty)

diff --git a/config/crd/bases/barbican.openstack.org_barbicans.yaml b/config/crd/bases/barbican.openstack.org_barbicans.yaml
@@ -48,10 +48,18 @@ spec:
           spec:
             description: BarbicanSpec defines the desired state of Barbican
             properties:
+              apiTimeout:
+                default: 90
+                description: Barbican API timeout
+                type: integer
               barbicanAPI:
                 description: BarbicanAPI - Spec definition for the  API services of
                   this Barbican deployment
                 properties:
+                  apiTimeout:
+                    description: APITimeout for HAProxy and Apache defaults to Barbican
+                      APITimeout (seconds)
+                    type: integer
                   containerImage:
                     description: ContainerImage - Barbican Container Image URL (will
                       be set to environmental default if empty)

diff --git a/controllers/barbican_controller.go b/controllers/barbican_controller.go
@@ -722,6 +722,9 @@ func (r *BarbicanReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, in
 		apiSpec.NodeSelector = instance.Spec.NodeSelector
 	}
 
+	// Note: The top-level .spec.apiTimeout ALWAYS overrides .spec.barbicanAPI.apiTimeout
+	apiSpec.BarbicanAPITemplate.APITimeout = instance.Spec.APITimeout
+
 	deployment := &barbicanv1beta1.BarbicanAPI{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-api", instance.Name),

diff --git a/controllers/barbicanapi_controller.go b/controllers/barbicanapi_controller.go
@@ -362,6 +362,7 @@ func (r *BarbicanAPIReconciler) generateServiceConfigs(
 		httpdVhostConfig[endpt.String()] = endptConfig
 	}
 	templateParameters["VHosts"] = httpdVhostConfig
+	templateParameters["TimeOut"] = instance.Spec.APITimeout
 
 	return GenerateConfigsGeneric(ctx, h, instance, envVars, templateParameters, customData, labels, false)
 }

diff --git a/templates/barbican/config/10-barbican_wsgi_main.conf b/templates/barbican/config/10-barbican_wsgi_main.conf
@@ -3,6 +3,7 @@
 # {{ $endpt }} vhost {{ $vhost.ServerName }} configuration
 <VirtualHost *:9311>
   ServerName {{ $vhost.ServerName }}
+  TimeOut {{ $.TimeOut }}
 
   ## Vhost docroot
   DocumentRoot "/var/www/cgi-bin/barbican"

diff --git a/tests/functional/barbican_controller_test.go b/tests/functional/barbican_controller_test.go
@@ -208,6 +208,15 @@ var _ = Describe("Barbican controller", func() {
 				corev1.ConditionTrue,
 			)
 		})
+
+		It("checks the 10-barbican_wsgi_main.conf contains the correct TimeOut", func() {
+			cf := th.GetSecret(barbicanTest.BarbicanAPIConfigSecret)
+			Expect(cf).ShouldNot(BeNil())
+			httpdConfData := string(cf.Data["10-barbican_wsgi_main.conf"])
+			Expect(httpdConfData).To(
+				ContainSubstring("TimeOut 90"),
+			)
+		})
 	})
 	When("A Barbican with TLS is created", func() {
 		BeforeEach(func() {

diff --git a/tests/functional/barbican_test_data.go b/tests/functional/barbican_test_data.go
@@ -64,6 +64,7 @@ type BarbicanTestData struct {
 	BarbicanServicePublic    types.NamespacedName
 	BarbicanServiceInternal  types.NamespacedName
 	BarbicanConfigSecret     types.NamespacedName
+	BarbicanAPIConfigSecret  types.NamespacedName
 	BarbicanConfigScripts    types.NamespacedName
 	BarbicanConfigMapData    types.NamespacedName
 	BarbicanScheduler        types.NamespacedName
@@ -137,6 +138,10 @@ func GetBarbicanTestData(barbicanName types.NamespacedName) BarbicanTestData {
 			Namespace: barbicanName.Namespace,
 			Name:      fmt.Sprintf("%s-%s", barbicanName.Name, "config-data"),
 		},
+		BarbicanAPIConfigSecret: types.NamespacedName{
+			Namespace: barbicanName.Namespace,
+			Name:      fmt.Sprintf("%s-%s", barbicanName.Name, "api-config-data"),
+		},
 		BarbicanConfigScripts: types.NamespacedName{
 			Namespace: barbicanName.Namespace,
 			Name:      fmt.Sprintf("%s-%s", barbicanName.Name, "scripts"),