Skip to content

Commit

Permalink
fix: correct response structure for GenerateConfig API
Browse files Browse the repository at this point in the history
Also fix recovery grpc handler to print panic stacktrace to the log.

Any API should follow the structure compatible with apid proxying
injection of errors/nodes.

Explicitly fail GenerateConfig API on worker nodes, as it panics on
worker nodes (missing certificates in node config).

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
  • Loading branch information
smira authored and talos-bot committed Feb 11, 2021
1 parent df00990 commit d99a016
Show file tree
Hide file tree
Showing 9 changed files with 445 additions and 325 deletions.
7 changes: 6 additions & 1 deletion api/machine/machine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -825,9 +825,14 @@ message GenerateConfigurationRequest {
MachineConfig machine_config = 3;
google.protobuf.Timestamp override_time = 4;
}

// GenerateConfiguration describes the response to a generate configuration request.
message GenerateConfigurationResponse {
message GenerateConfiguration {
common.Metadata metadata = 1;
repeated bytes data = 2;
bytes talosconfig = 3;
}

message GenerateConfigurationResponse {
repeated GenerateConfiguration messages = 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,11 @@ func (s *Server) ApplyConfiguration(ctx context.Context, in *machine.ApplyConfig
}

// GenerateConfiguration implements the machine.MachineServer interface.
// nolint:gocyclo
func (s *Server) GenerateConfiguration(ctx context.Context, in *machine.GenerateConfigurationRequest) (reply *machine.GenerateConfigurationResponse, err error) {
if s.Controller.Runtime().Config().Machine().Type() == machinetype.TypeJoin {
return nil, fmt.Errorf("config can't be generated on worker nodes")
}

return configuration.Generate(ctx, in)
}

Expand Down
14 changes: 9 additions & 5 deletions internal/integration/api/generate-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/talos-systems/talos/internal/integration/base"
"github.com/talos-systems/talos/pkg/images"
machineapi "github.com/talos-systems/talos/pkg/machinery/api/machine"
"github.com/talos-systems/talos/pkg/machinery/client"
clientconfig "github.com/talos-systems/talos/pkg/machinery/client/config"
"github.com/talos-systems/talos/pkg/machinery/config/configloader"
"github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1/machine"
Expand Down Expand Up @@ -78,14 +79,17 @@ func (suite *GenerateConfigSuite) TestGenerate() {
},
}

node := suite.RandomDiscoveredNode(machine.TypeControlPlane)
ctx := client.WithNodes(suite.ctx, node)

reply, err := suite.Client.GenerateConfiguration(
suite.ctx,
ctx,
request,
)

suite.Require().NoError(err)

data := reply.GetData()
data := reply.Messages[0].GetData()

config, err := configloader.NewFromBytes(data[0])

Expand All @@ -104,7 +108,7 @@ func (suite *GenerateConfigSuite) TestGenerate() {
suite.Require().EqualValues(request.MachineConfig.NetworkConfig.Hostname, config.Machine().Network().Hostname())
suite.Require().EqualValues(request.MachineConfig.NetworkConfig.Hostname, config.Machine().Network().Hostname())

talosconfig, err := clientconfig.FromBytes(reply.Talosconfig)
talosconfig, err := clientconfig.FromBytes(reply.Messages[0].Talosconfig)

suite.Require().NoError(err)

Expand All @@ -122,13 +126,13 @@ func (suite *GenerateConfigSuite) TestGenerate() {
request.MachineConfig.Type = machineapi.MachineConfig_MachineType(machine.TypeControlPlane)

reply, err = suite.Client.GenerateConfiguration(
suite.ctx,
ctx,
request,
)

suite.Require().NoError(err)

data = reply.GetData()
data = reply.Messages[0].GetData()

joinedConfig, err := configloader.NewFromBytes(data[0])

Expand Down
8 changes: 6 additions & 2 deletions internal/pkg/configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,12 @@ func Generate(ctx context.Context, in *machine.GenerateConfigurationRequest) (re
}

reply = &machine.GenerateConfigurationResponse{
Data: [][]byte{cfgBytes},
Talosconfig: taloscfgBytes,
Messages: []*machine.GenerateConfiguration{
{
Data: [][]byte{cfgBytes},
Talosconfig: taloscfgBytes,
},
},
}
default:
return nil, fmt.Errorf("unsupported config version %s", in.ConfigVersion)
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/tui/installer/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,8 @@ func (installer *Installer) apply(conn *Connection) error {
return err
}

config = response.Data[0]
talosconfig, err = clientconfig.FromBytes(response.Talosconfig)
config = response.Messages[0].Data[0]
talosconfig, err = clientconfig.FromBytes(response.Messages[0].Talosconfig)

if err != nil {
return err
Expand Down
21 changes: 18 additions & 3 deletions pkg/grpc/factory/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ import (
"net"
"os"
"path/filepath"
"runtime/debug"
"strconv"

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

grpclog "github.com/talos-systems/talos/pkg/grpc/middleware/log"
)
Expand Down Expand Up @@ -108,6 +111,16 @@ func WithDefaultLog() Option {
}
}

func recoveryHandler(logger *log.Logger) grpc_recovery.RecoveryHandlerFunc {
return func(p interface{}) error {
if logger != nil {
logger.Printf("panic:\n%s", string(debug.Stack()))
}

return status.Errorf(codes.Internal, "%v", p)
}
}

// NewDefaultOptions initializes the Options struct with default values.
func NewDefaultOptions(setters ...Option) *Options {
opts := &Options{
Expand All @@ -119,8 +132,10 @@ func NewDefaultOptions(setters ...Option) *Options {
setter(opts)
}

var logger *log.Logger

if opts.LogDestination != nil {
logger := log.New(opts.LogDestination, opts.LogPrefix, log.Flags())
logger = log.New(opts.LogDestination, opts.LogPrefix, log.Flags())

logMiddleware := grpclog.NewMiddleware(logger)

Expand All @@ -134,8 +149,8 @@ func NewDefaultOptions(setters ...Option) *Options {
// Install default recovery interceptors.
// Recovery is installed as the last middleware in the chain so that earlier middlewares in the chain
// have a chance to process the error (e.g. logging middleware).
opts.StreamInterceptors = append(opts.StreamInterceptors, grpc_recovery.StreamServerInterceptor())
opts.UnaryInterceptors = append(opts.UnaryInterceptors, grpc_recovery.UnaryServerInterceptor())
opts.StreamInterceptors = append(opts.StreamInterceptors, grpc_recovery.StreamServerInterceptor(grpc_recovery.WithRecoveryHandler(recoveryHandler(logger))))
opts.UnaryInterceptors = append(opts.UnaryInterceptors, grpc_recovery.UnaryServerInterceptor(grpc_recovery.WithRecoveryHandler(recoveryHandler(logger))))

opts.ServerOptions = append(opts.ServerOptions,
grpc.StreamInterceptor(grpc_middleware.ChainStreamServer(opts.StreamInterceptors...)),
Expand Down
Loading

0 comments on commit d99a016

Please sign in to comment.