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

[BUG] [sync-service] not supporting SSL #1479

Open
aepfli opened this issue Dec 18, 2024 · 0 comments
Open

[BUG] [sync-service] not supporting SSL #1479

aepfli opened this issue Dec 18, 2024 · 0 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed Needs Triage This issue needs to be investigated by a maintainer

Comments

@aepfli
Copy link
Member

aepfli commented Dec 18, 2024

Observed behavior

It is not possible to connect to the sync service via SSL; the same configuration works fine via the Evaluation Service.

see: https://github.com/open-feature/java-sdk-contrib/pull/1111/files#diff-ea9c090ae5dbf03273dc224b296f7a665d6d715d13e8d36e2b420acc6f10b030 (java e2e test implementation, trying to test SSL, currently disabled due to missing functionality)

Expected Behavior

Connection to the Sync Service should be possible to be made via SSL. and the configuration defined in the spec should be honored (here)

Steps to reproduce

open-feature/java-sdk-contrib#1111 is a running Gherkin test against the SSL image from the flagd-testbed

devnotes

Within the evaluation service we are starting to serve via:

if svcConf.CertPath != "" && svcConf.KeyPath != "" {
if err := s.server.ServeTLS(
lis,
svcConf.CertPath,
svcConf.KeyPath,
); err != nil && !errors.Is(err, http.ErrServerClosed) {
return fmt.Errorf("error returned from flag evaluation server: %w", err)
}
} else {
if err := s.server.Serve(
lis,
); err != nil && !errors.Is(err, http.ErrServerClosed) {
return fmt.Errorf("error returned from flag evaluation server: %w", err)
}
}
return nil

Whereas in the sync service, we don't differentiate:

func (s *Service) Start(ctx context.Context) error {
// derive errgroup so we track ctx for exit as well as startup errors
g, lCtx := errgroup.WithContext(ctx)
g.Go(func() error {
// delay server start until we see all syncs from known sync sources OR timeout
select {
case <-time.After(5 * time.Second):
s.logger.Warn("timeout while waiting for all sync sources to complete their initial sync. " +
"continuing sync service")
break
case <-s.startupTracker.getDone():
break
}
err := s.server.Serve(s.listener)
if err != nil {
s.logger.Warn(fmt.Sprintf("error from sync server start: %v", err))
}
return nil
})
g.Go(func() error {
<-lCtx.Done()
s.shutdown()
return nil
})
err := g.Wait()
if err != nil {
return fmt.Errorf("error from sync service: %w", err)
}
return nil

@aepfli aepfli added bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer labels Dec 18, 2024
@toddbaert toddbaert added good first issue Good for newcomers help wanted Extra attention is needed labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed Needs Triage This issue needs to be investigated by a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants