From ca5952fe3484d6af9f21340a2e1147affcc06037 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 23 Jun 2022 13:17:57 +0200 Subject: [PATCH 1/3] Improve LDAP CA cert check The check was still racy as it could return early if the cert file exists but was not fully written yet. --- ocis-pkg/ldap/ldap.go | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/ocis-pkg/ldap/ldap.go b/ocis-pkg/ldap/ldap.go index a03d58848..8eceaf3cb 100644 --- a/ocis-pkg/ldap/ldap.go +++ b/ocis-pkg/ldap/ldap.go @@ -1,24 +1,39 @@ package ldap import ( + "crypto/x509" "errors" + "io/ioutil" "os" "time" "github.com/owncloud/ocis/v2/ocis-pkg/log" ) -const _caTimeout = 5 +const ( + caCheckRetries = 3 + caCheckSleep = 2 +) func WaitForCA(log log.Logger, insecure bool, caCert string) error { if !insecure && caCert != "" { - if _, err := os.Stat(caCert); errors.Is(err, os.ErrNotExist) { - log.Warn().Str("LDAP CACert", caCert).Msgf("File does not exist. Waiting %d seconds for it to appear.", _caTimeout) - time.Sleep(_caTimeout * time.Second) - if _, err := os.Stat(caCert); errors.Is(err, os.ErrNotExist) { - log.Warn().Str("LDAP CACert", caCert).Msgf("File still does not exist after Timeout") + for i := 0; i < caCheckRetries; i++ { + if _, err := os.Stat(caCert); err != nil && !errors.Is(err, os.ErrNotExist) { return err } + // Check if this actually is a CA cert. We need to retry here as well + // as the file might exist already, but have no contents yet. + certs := x509.NewCertPool() + pemData, err := ioutil.ReadFile(caCert) + if err != nil { + log.Debug().Err(err).Str("LDAP CACert", caCert).Msg("Error reading CA") + } else if !certs.AppendCertsFromPEM(pemData) { + log.Debug().Str("LDAP CAcert", caCert).Msg("Failed to append CA to pool") + } else { + return nil + } + time.Sleep(caCheckSleep * time.Second) + log.Warn().Str("LDAP CACert", caCert).Msgf("CA cert file is not ready yet. Waiting %d seconds for it to appear.", caCheckSleep) } } return nil From 917f099751e6a5e041a814e8dccbd21484cb59d7 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 23 Jun 2022 13:19:25 +0200 Subject: [PATCH 2/3] Error out if LDAP CA cert is not valid If the configured LDAP CA cert can not be successfully loaded to the Pool let the creation of the Graph Service fail. --- extensions/graph/pkg/server/http/server.go | 4 ++++ extensions/graph/pkg/service/v0/service.go | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/extensions/graph/pkg/server/http/server.go b/extensions/graph/pkg/server/http/server.go index d61a17f6a..ed208ad22 100644 --- a/extensions/graph/pkg/server/http/server.go +++ b/extensions/graph/pkg/server/http/server.go @@ -59,6 +59,10 @@ func Server(opts ...Option) (http.Service, error) { svc.EventsPublisher(publisher), ) + if handle == nil { + return http.Service{}, errors.New("could not initialize graph service") + } + { handle = svc.NewInstrument(handle, options.Metrics) handle = svc.NewLogging(handle, options.Logger) diff --git a/extensions/graph/pkg/service/v0/service.go b/extensions/graph/pkg/service/v0/service.go index 2386a21c6..ee31660a8 100644 --- a/extensions/graph/pkg/service/v0/service.go +++ b/extensions/graph/pkg/service/v0/service.go @@ -106,10 +106,13 @@ func NewService(opts ...Option) Service { certs := x509.NewCertPool() pemData, err := ioutil.ReadFile(options.Config.Identity.LDAP.CACert) if err != nil { - options.Logger.Error().Msgf("Error initializing LDAP Backend: '%s'", err) + options.Logger.Error().Err(err).Msgf("Error initializing LDAP Backend") + return nil + } + if !certs.AppendCertsFromPEM(pemData) { + options.Logger.Error().Msgf("Error initializing LDAP Backend. Adding CA cert failed") return nil } - certs.AppendCertsFromPEM(pemData) tlsConf.RootCAs = certs } From b2c304c5d8c93d0b009df66834e43100c437a5af Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 23 Jun 2022 13:21:33 +0200 Subject: [PATCH 3/3] Reduce the number of retries for `wait-for-ocis-server` `curl` retries with exponantional backoff. 10 retries mean more than a total of 16min wait time until we fail. This seems far too long. With 7 retries we should be at a bit more than 2 minutes max, if ocis takes that long to start, something is likely broken in the infrastructure. --- .drone.star | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.star b/.drone.star index 9e0698e5d..4426a3c7c 100644 --- a/.drone.star +++ b/.drone.star @@ -1593,7 +1593,7 @@ def ocisServer(storage, accounts_hash_difficulty = 4, volumes = [], depends_on = "name": "wait-for-ocis-server", "image": OC_CI_ALPINE, "commands": [ - "curl -k -u admin:admin --fail --retry-connrefused --retry 10 --retry-all-errors 'https://ocis-server:9200/graph/v1.0/users/admin'", + "curl -k -u admin:admin --fail --retry-connrefused --retry 7 --retry-all-errors 'https://ocis-server:9200/graph/v1.0/users/admin'", ], "depends_on": depends_on, }