Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurable API Timeout #197

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/bases/barbican.openstack.org_barbicanapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions api/bases/barbican.openstack.org_barbicans.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions api/v1beta1/barbican_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 36 additions & 6 deletions api/v1beta1/barbican_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type BarbicanDefaults struct {
APIContainerImageURL string
WorkerContainerImageURL string
KeystoneListenerContainerImageURL string
BarbicanAPITimeout int
}

var barbicanDefaults BarbicanDefaults
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
4 changes: 4 additions & 0 deletions api/v1beta1/barbicanapi_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/barbican.openstack.org_barbicanapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/barbican.openstack.org_barbicans.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions controllers/barbican_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows other operators behavior

apiSpec.BarbicanAPITemplate.APITimeout = instance.Spec.APITimeout

deployment := &barbicanv1beta1.BarbicanAPI{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-api", instance.Name),
Expand Down
1 change: 1 addition & 0 deletions controllers/barbicanapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions templates/barbican/config/10-barbican_wsgi_main.conf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# {{ $endpt }} vhost {{ $vhost.ServerName }} configuration
<VirtualHost *:9311>
ServerName {{ $vhost.ServerName }}
TimeOut {{ $.TimeOut }}

## Vhost docroot
DocumentRoot "/var/www/cgi-bin/barbican"
Expand Down
9 changes: 9 additions & 0 deletions tests/functional/barbican_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 5 additions & 0 deletions tests/functional/barbican_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"),
Expand Down
Loading