From 67ac6933d3c23b8ea31f01bd45d0192573e64ef3 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 23 Feb 2024 17:38:56 +0400 Subject: [PATCH] fix: handle errors to watch apid/trustd certs Fixes #8345 Both `apid` and `trustd` services use a gRPC connection back to `machined` to watch changes to the certificates (new certificates being issued). This refactors the code to follow regular conventions, so that a failure to watch will crash the process, and they have a way to restart and re-establish the watch. Use the context and errgroup consistently. Signed-off-by: Andrey Smirnov --- internal/app/apid/main.go | 6 +- internal/app/apid/pkg/provider/provider.go | 70 +++++++++++-------- .../app/trustd/internal/provider/provider.go | 68 ++++++++++-------- internal/app/trustd/main.go | 7 +- 4 files changed, 90 insertions(+), 61 deletions(-) diff --git a/internal/app/apid/main.go b/internal/app/apid/main.go index 0e43d7510..91e83c9c1 100644 --- a/internal/app/apid/main.go +++ b/internal/app/apid/main.go @@ -81,7 +81,7 @@ func apidMain() error { stateClient := v1alpha1.NewStateClient(runtimeConn) resources := state.WrapCore(client.NewAdapter(stateClient)) - tlsConfig, err := provider.NewTLSConfig(resources) + tlsConfig, err := provider.NewTLSConfig(ctx, resources) if err != nil { return fmt.Errorf("failed to create remote certificate provider: %w", err) } @@ -225,6 +225,10 @@ func apidMain() error { return socketServer.Serve(socketListener) }) + errGroup.Go(func() error { + return tlsConfig.Watch(ctx) + }) + errGroup.Go(func() error { <-ctx.Done() diff --git a/internal/app/apid/pkg/provider/provider.go b/internal/app/apid/pkg/provider/provider.go index 420b89d45..abbd62830 100644 --- a/internal/app/apid/pkg/provider/provider.go +++ b/internal/app/apid/pkg/provider/provider.go @@ -9,7 +9,6 @@ import ( "context" stdlibtls "crypto/tls" "fmt" - "log" "sync" "github.com/cosi-project/runtime/pkg/resource" @@ -22,15 +21,14 @@ import ( // TLSConfig provides client & server TLS configs for apid. type TLSConfig struct { certificateProvider *certificateProvider + watchCh <-chan state.Event } // NewTLSConfig builds provider from configuration and endpoints. -// -//nolint:gocyclo -func NewTLSConfig(resources state.State) (*TLSConfig, error) { +func NewTLSConfig(ctx context.Context, resources state.State) (*TLSConfig, error) { watchCh := make(chan state.Event) - if err := resources.Watch(context.TODO(), resource.NewMetadata(secrets.NamespaceName, secrets.APIType, secrets.APIID, resource.VersionUndefined), watchCh); err != nil { + if err := resources.Watch(ctx, resource.NewMetadata(secrets.NamespaceName, secrets.APIType, secrets.APIID, resource.VersionUndefined), watchCh); err != nil { return nil, fmt.Errorf("error setting up watch: %w", err) } @@ -38,7 +36,13 @@ func NewTLSConfig(resources state.State) (*TLSConfig, error) { provider := &certificateProvider{} for { - event := <-watchCh + var event state.Event + + select { + case <-ctx.Done(): + return nil, ctx.Err() + case event = <-watchCh: + } switch event.Type { case state.Created, state.Updated: @@ -56,36 +60,40 @@ func NewTLSConfig(resources state.State) (*TLSConfig, error) { return nil, err } - break + return &TLSConfig{ + certificateProvider: provider, + watchCh: watchCh, + }, nil } +} - go func() { - for { - event := <-watchCh +// Watch for changes in API certificates and updates the TLSConfig. +func (tlsConfig *TLSConfig) Watch(ctx context.Context) error { + for { + var event state.Event - switch event.Type { - case state.Created, state.Updated: - // expected - case state.Destroyed, state.Bootstrapped: - // ignore, we'll get another event - continue - case state.Errored: - log.Printf("error watching for API certificates: %s", event.Error) - - continue - } - - apiCerts := event.Resource.(*secrets.API) //nolint:errcheck,forcetypeassert - - if err := provider.Update(apiCerts); err != nil { - log.Printf("failed updating cert: %v", err) - } + select { + case <-ctx.Done(): + return nil + case event = <-tlsConfig.watchCh: } - }() - return &TLSConfig{ - certificateProvider: provider, - }, nil + switch event.Type { + case state.Created, state.Updated: + // expected + case state.Destroyed, state.Bootstrapped: + // ignore, we'll get another event + continue + case state.Errored: + return fmt.Errorf("error watching API certificates: %w", event.Error) + } + + apiCerts := event.Resource.(*secrets.API) //nolint:errcheck,forcetypeassert + + if err := tlsConfig.certificateProvider.Update(apiCerts); err != nil { + return fmt.Errorf("failed updating cert: %v", err) + } + } } // ServerConfig generates server-side tls.Config. diff --git a/internal/app/trustd/internal/provider/provider.go b/internal/app/trustd/internal/provider/provider.go index da27eac48..2dfcd7f78 100644 --- a/internal/app/trustd/internal/provider/provider.go +++ b/internal/app/trustd/internal/provider/provider.go @@ -22,15 +22,15 @@ import ( // TLSConfig provides client & server TLS configs for trustd. type TLSConfig struct { certificateProvider *certificateProvider + + watchCh <-chan state.Event } // NewTLSConfig builds provider from configuration and endpoints. -// -//nolint:gocyclo -func NewTLSConfig(resources state.State) (*TLSConfig, error) { +func NewTLSConfig(ctx context.Context, resources state.State) (*TLSConfig, error) { watchCh := make(chan state.Event) - if err := resources.Watch(context.TODO(), resource.NewMetadata(secrets.NamespaceName, secrets.TrustdType, secrets.TrustdID, resource.VersionUndefined), watchCh); err != nil { + if err := resources.Watch(ctx, resource.NewMetadata(secrets.NamespaceName, secrets.TrustdType, secrets.TrustdID, resource.VersionUndefined), watchCh); err != nil { return nil, fmt.Errorf("error setting up watch: %w", err) } @@ -38,7 +38,13 @@ func NewTLSConfig(resources state.State) (*TLSConfig, error) { provider := &certificateProvider{} for { - event := <-watchCh + var event state.Event + + select { + case <-ctx.Done(): + return nil, ctx.Err() + case event = <-watchCh: + } switch event.Type { case state.Created, state.Updated: @@ -56,34 +62,40 @@ func NewTLSConfig(resources state.State) (*TLSConfig, error) { return nil, err } - break + return &TLSConfig{ + certificateProvider: provider, + watchCh: watchCh, + }, nil } +} - go func() { - for { - event := <-watchCh +// Watch for updates to trustd certificates. +func (tlsConfig *TLSConfig) Watch(ctx context.Context) error { + for { + var event state.Event - switch event.Type { - case state.Created, state.Updated: - // expected - case state.Destroyed, state.Bootstrapped: - // ignore, we'll get another event - continue - case state.Errored: - log.Printf("error watching for trustd certificates: %s", event.Error) - } - - trustdCerts := event.Resource.(*secrets.Trustd) //nolint:errcheck,forcetypeassert - - if err := provider.Update(trustdCerts); err != nil { - log.Printf("failed updating cert: %v", err) - } + select { + case <-ctx.Done(): + return nil + case event = <-tlsConfig.watchCh: } - }() - return &TLSConfig{ - certificateProvider: provider, - }, nil + switch event.Type { + case state.Created, state.Updated: + // expected + case state.Destroyed, state.Bootstrapped: + // ignore, we'll get another event + continue + case state.Errored: + log.Printf("error watching for trustd certificates: %s", event.Error) + } + + trustdCerts := event.Resource.(*secrets.Trustd) //nolint:errcheck,forcetypeassert + + if err := tlsConfig.certificateProvider.Update(trustdCerts); err != nil { + return fmt.Errorf("failed updating cert: %w", err) + } + } } // ServerConfig generates server-side tls.Config. diff --git a/internal/app/trustd/main.go b/internal/app/trustd/main.go index 12eb070be..69345c9f8 100644 --- a/internal/app/trustd/main.go +++ b/internal/app/trustd/main.go @@ -2,6 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Package trustd implements trustd functionality. package trustd import ( @@ -78,7 +79,7 @@ func trustdMain() error { stateClient := v1alpha1.NewStateClient(runtimeConn) resources := state.WrapCore(client.NewAdapter(stateClient)) - tlsConfig, err := provider.NewTLSConfig(resources) + tlsConfig, err := provider.NewTLSConfig(ctx, resources) if err != nil { return fmt.Errorf("failed to create remote certificate provider: %w", err) } @@ -114,6 +115,10 @@ func trustdMain() error { return networkServer.Serve(networkListener) }) + errGroup.Go(func() error { + return tlsConfig.Watch(ctx) + }) + errGroup.Go(func() error { <-ctx.Done()