alt-orchestra/talos
1
0
Fork 0
Code Issues Pull Requests Actions 27 Packages Projects Releases Wiki Activity

feat: add config validation warnings

Browse Source 
Closes #3412.
Refs #3413.

Signed-off-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com>
This commit is contained in:
Alexey Palazhchenko 2021-04-08 11:42:43 +00:00 committed by talos-bot
parent eee7ad13aa
commit 29da22d063
17 changed files with 1331 additions and 1184 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api/machine/machine.proto
View File

@ -94,7 +94,11 @@ message ApplyConfigurationRequest {
}
// ApplyConfigurationResponse describes the response to a configuration request.
message ApplyConfiguration { common.Metadata metadata = 1; }
message ApplyConfiguration {
common.Metadata metadata = 1;
// Configuration validation warnings.
repeated string warnings = 2;
}
message ApplyConfigurationResponse { repeated ApplyConfiguration messages = 1; }
// rpc reboot

14
cmd/talosctl/cmd/mgmt/validate.go
View File

@ -18,6 +18,7 @@ import (
var (
validateConfigArg string
validateModeArg string
validateStrictArg bool
)
// validateCmd reads in a userData file and attempts to parse it.
@ -36,7 +37,17 @@ var validateCmd = &cobra.Command{
if err != nil {
return err
}
if err := cfg.Validate(mode, config.WithLocal()); err != nil {
opts := []config.ValidationOption{config.WithLocal()}
if validateStrictArg {
opts = append(opts, config.WithStrict())
}
warnings, err := cfg.Validate(mode, opts...)
for _, w := range warnings {
cli.Warning("%s", w)
}
if err != nil {
return err
}
@ -56,5 +67,6 @@ func init() {
fmt.Sprintf("the mode to validate the config for (valid values are %s, %s, and %s)", runtime.ModeMetal.String(), runtime.ModeCloud.String(), runtime.ModeContainer.String()),
)
cli.Should(validateCmd.MarkFlagRequired("mode"))
validateCmd.Flags().BoolVarP(&validateStrictArg, "strict", "", false, "treat validation warnings as errors")
addCommand(validateCmd)
}

11
cmd/talosctl/cmd/talos/apply-config.go
View File

@ -15,6 +15,7 @@ import (
"github.com/talos-systems/crypto/x509"
"github.com/talos-systems/talos/internal/pkg/tui/installer"
"github.com/talos-systems/talos/pkg/cli"
machineapi "github.com/talos-systems/talos/pkg/machinery/api/machine"
"github.com/talos-systems/talos/pkg/machinery/client"
)
@ -148,11 +149,17 @@ var applyConfigCmd = &cobra.Command{
return install.Run(conn)
}
if _, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
resp, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
Data: cfgBytes,
OnReboot: applyConfigCmdFlags.onReboot,
Immediate: applyConfigCmdFlags.immediate,
}); err != nil {
})
for _, m := range resp.GetMessages() {
for _, w := range m.GetWarnings() {
cli.Warning("%s", w)
}
}
if err != nil {
return fmt.Errorf("error applying new configuration: %s", err)
}

8
cmd/talosctl/cmd/talos/edit.go
View File

@ -19,6 +19,7 @@ import (
"k8s.io/kubectl/pkg/cmd/util/editor/crlf"
"github.com/talos-systems/talos/cmd/talosctl/pkg/talos/helpers"
"github.com/talos-systems/talos/pkg/cli"
"github.com/talos-systems/talos/pkg/machinery/api/machine"
"github.com/talos-systems/talos/pkg/machinery/client"
"github.com/talos-systems/talos/pkg/resources/config"
@ -130,7 +131,7 @@ or 'notepad' for Windows.`,
break
}
_, err = c.ApplyConfiguration(parentCtx, &machine.ApplyConfigurationRequest{
resp, err := c.ApplyConfiguration(parentCtx, &machine.ApplyConfigurationRequest{
Data: edited,
Immediate: editCmdFlags.immediate,
OnReboot: editCmdFlags.onReboot,
@ -140,6 +141,11 @@ or 'notepad' for Windows.`,
continue
}
for _, m := range resp.GetMessages() {
for _, w := range m.GetWarnings() {
cli.Warning("%s", w)
}
}
break
}

9
cmd/talosctl/cmd/talos/patch.go
View File

@ -18,6 +18,7 @@ import (
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"github.com/talos-systems/talos/cmd/talosctl/pkg/talos/helpers"
"github.com/talos-systems/talos/pkg/cli"
"github.com/talos-systems/talos/pkg/machinery/api/machine"
"github.com/talos-systems/talos/pkg/machinery/client"
"github.com/talos-systems/talos/pkg/machinery/config/configpatcher"
@ -85,7 +86,7 @@ var patchCmd = &cobra.Command{
return err
}
_, err = c.ApplyConfiguration(ctx, &machine.ApplyConfigurationRequest{
resp, err := c.ApplyConfiguration(ctx, &machine.ApplyConfigurationRequest{
Data: patched,
Immediate: patchCmdFlags.immediate,
OnReboot: patchCmdFlags.onReboot,
@ -102,6 +103,12 @@ var patchCmd = &cobra.Command{
fmt.Printf("patched %s at the node %s\n", args[0], msg.Metadata.GetHostname())
for _, m := range resp.GetMessages() {
for _, w := range m.GetWarnings() {
cli.Warning("%s", w)
}
}
return err
}

2
internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_runtime.go
View File

@ -49,7 +49,7 @@ func (r *Runtime) ValidateConfig(b []byte) (config.Provider, error) {
return nil, fmt.Errorf("failed to parse config: %w", err)
}
if err := cfg.Validate(r.State().Platform().Mode()); err != nil {
if _, err := cfg.Validate(r.State().Platform().Mode()); err != nil {
return nil, fmt.Errorf("failed to validate config: %w", err)
}

14
internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go
View File

@ -539,7 +539,12 @@ func receiveConfigViaMaintenanceService(ctx context.Context, logger *log.Logger,
return nil, fmt.Errorf("failed to create config provider: %w", err)
}
if err = provider.Validate(r.State().Platform().Mode()); err != nil {
warnings, err := provider.Validate(r.State().Platform().Mode())
for _, w := range warnings {
logger.Printf("WARNING:\n%s", w)
}
if err != nil {
return nil, fmt.Errorf("failed to validate config: %w", err)
}
@ -554,7 +559,12 @@ func receiveConfigViaMaintenanceService(ctx context.Context, logger *log.Logger,
// ValidateConfig validates the config.
func ValidateConfig(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, string) {
return func(ctx context.Context, logger *log.Logger, r runtime.Runtime) error {
return r.Config().Validate(r.State().Platform().Mode())
warnings, err := r.Config().Validate(r.State().Platform().Mode())
for _, w := range warnings {
logger.Printf("WARNING:\n%s", w)
}
return err
}, "validateConfig"
}

7
internal/app/maintenance/server/server.go
View File

@ -61,13 +61,16 @@ func (s *Server) ApplyConfiguration(ctx context.Context, in *machine.ApplyConfig
return nil, fmt.Errorf("failed to parse config: %w", err)
}
if err = cfgProvider.Validate(s.runtime.State().Platform().Mode()); err != nil {
warnings, err := cfgProvider.Validate(s.runtime.State().Platform().Mode())
if err != nil {
return nil, fmt.Errorf("configuration validation failed: %w", err)
}
reply = &machine.ApplyConfigurationResponse{
Messages: []*machine.ApplyConfiguration{
{},
{
Warnings: warnings,
},
},
}

2
internal/integration/cli/validate.go
View File

@ -61,7 +61,7 @@ func (suite *ValidateSuite) TestValidate() {
mode := mode
suite.Run(fmt.Sprintf("%s-%s", configFile, mode), func() {
suite.RunCLI([]string{"validate", "-m", mode, "-c", configFile})
suite.RunCLI([]string{"validate", "-m", mode, "-c", configFile, "--strict"})
})
}
}

2313
pkg/machinery/api/machine/machine.pb.go
View File

File diff suppressed because it is too large Load Diff

3
pkg/machinery/config/provider.go
View File

@ -26,7 +26,8 @@ type Provider interface {
Persist() bool
Machine() MachineConfig
Cluster() ClusterConfig
Validate(RuntimeMode, ...ValidationOption) error
// Validate checks configuration and returns warnings and fatal errors (as multierror).
Validate(RuntimeMode, ...ValidationOption) ([]string, error)
ApplyDynamicConfig(context.Context, DynamicConfigProvider) error
String(encoderOptions ...encoder.Option) (string, error)
Bytes(encoderOptions ...encoder.Option) ([]byte, error)

1
pkg/machinery/config/types/v1alpha1/v1alpha1_provider.go
View File

@ -218,6 +218,7 @@ func (m *MachineConfig) Files() ([]config.File, error) {
}
// Type implements the config.Provider interface.
// TODO use machine.ParseType? https://github.com/talos-systems/talos/issues/3413
func (m *MachineConfig) Type() machine.Type {
switch m.MachineType {
case "init":

27
pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go
View File

@ -55,17 +55,20 @@ var (
// NetworkDeviceCheck defines the function type for checks.
type NetworkDeviceCheck func(*Device) error
// Validate implements the Configurator interface.
// Validate implements the config.Provider interface.
//nolint:gocyclo,cyclop
func (c *Config) Validate(mode config.RuntimeMode, options ...config.ValidationOption) error {
var result *multierror.Error
func (c *Config) Validate(mode config.RuntimeMode, options ...config.ValidationOption) ([]string, error) {
var (
warnings []string
result *multierror.Error
)
opts := config.NewValidationOptions(options...)
if c.MachineConfig == nil {
result = multierror.Append(result, errors.New("machine instructions are required"))
return result.ErrorOrNil()
return nil, result.ErrorOrNil()
}
if err := c.ClusterConfig.Validate(); err != nil {
@ -98,6 +101,12 @@ func (c *Config) Validate(mode config.RuntimeMode, options ...config.ValidationO
}
}
// TODO rework machine type validation https://github.com/talos-systems/talos/issues/3413
if c.MachineConfig.MachineType == "" {
warnings = append(warnings, `machine type is empty`)
}
if c.Machine().Type() == machine.TypeInit || c.Machine().Type() == machine.TypeControlPlane {
switch c.Cluster().Network().CNI().Name() {
case constants.CustomCNI:
@ -157,7 +166,15 @@ func (c *Config) Validate(mode config.RuntimeMode, options ...config.ValidationO
}
}
return result.ErrorOrNil()
if opts.Strict {
for _, w := range warnings {
result = multierror.Append(result, fmt.Errorf("warning: %s", w))
}
warnings = nil
}
return warnings, result.ErrorOrNil()
}
// Validate validates the config.

85
pkg/machinery/config/types/v1alpha1/v1alpha1_validation_test.go
View File

@ -35,10 +35,12 @@ func TestValidate(t *testing.T) {
require.NoError(t, err)
for _, test := range []struct {
name string
config *v1alpha1.Config
requiresInstall bool
expectedError string
name string
config *v1alpha1.Config
requiresInstall bool
strict bool
expectedWarnings []string
expectedError string
}{
{
name: "NoMachine",
@ -48,7 +50,7 @@ func TestValidate(t *testing.T) {
expectedError: "1 error occurred:\n\t* machine instructions are required\n\n",
},
{
name: "NoMachineInstall",
name: "NoMachineType",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
@ -60,12 +62,49 @@ func TestValidate(t *testing.T) {
},
},
},
expectedWarnings: []string{
`machine type is empty`,
},
},
{
name: "NoMachineTypeStrict",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
endpointURL,
},
},
},
},
strict: true,
expectedError: "1 error occurred:\n\t* warning: machine type is empty\n\n",
},
{
name: "NoMachineInstall",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
endpointURL,
},
},
},
},
},
{
name: "NoMachineInstallRequired",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -82,6 +121,7 @@ func TestValidate(t *testing.T) {
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
MachineInstall: &v1alpha1.InstallConfig{
InstallDisk: "/dev/vda",
},
@ -101,7 +141,9 @@ func TestValidate(t *testing.T) {
name: "ExternalCloudProviderEnabled",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -122,7 +164,9 @@ func TestValidate(t *testing.T) {
name: "ExternalCloudProviderEnabledNoManifests",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -139,7 +183,9 @@ func TestValidate(t *testing.T) {
name: "ExternalCloudProviderDisabled",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -154,7 +200,9 @@ func TestValidate(t *testing.T) {
name: "ExternalCloudProviderExtraManifests",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -175,7 +223,9 @@ func TestValidate(t *testing.T) {
name: "ExternalCloudProviderInvalidManifests",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "join",
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
@ -198,12 +248,19 @@ func TestValidate(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
err := test.config.Validate(runtimeMode{test.requiresInstall}, config.WithLocal())
opts := []config.ValidationOption{config.WithLocal()}
if test.strict {
opts = append(opts, config.WithStrict())
}
warnings, errrors := test.config.Validate(runtimeMode{test.requiresInstall}, opts...)
assert.Equal(t, test.expectedWarnings, warnings)
if test.expectedError == "" {
assert.NoError(t, err)
assert.NoError(t, errrors)
} else {
assert.EqualError(t, err, test.expectedError)
assert.EqualError(t, errrors, test.expectedError)
}
})
}

11
pkg/machinery/config/validation.go
View File

@ -8,6 +8,8 @@ package config
type ValidationOptions struct {
// Local should disable part of the validation flow which won't work on the host machine.
Local bool
// Strict mode returns warnings as errors.
Strict bool
}
// ValidationOption represents an additional validation parameter for the config Validate method.
@ -23,9 +25,16 @@ func NewValidationOptions(options ...ValidationOption) *ValidationOptions {
return opts
}
// WithLocal enable local flag.
// WithLocal enables local flag.
func WithLocal() ValidationOption {
return func(opts *ValidationOptions) {
opts.Local = true
}
}
// WithStrict enables strict flag.
func WithStrict() ValidationOption {
return func(opts *ValidationOptions) {
opts.Strict = true
}
}

1
website/content/docs/v0.10/Reference/api.md
View File

@ -610,6 +610,7 @@ ApplyConfigurationResponse describes the response to a configuration request.
| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| metadata | [common.Metadata](#common.Metadata) | | |
| warnings | [string](#string) | repeated | Configuration validation warnings. |

1
website/content/docs/v0.10/Reference/cli.md
View File

@ -2019,6 +2019,7 @@ talosctl validate [flags]
-c, --config string the path of the config file
-h, --help help for validate
-m, --mode string the mode to validate the config for (valid values are metal, cloud, and container)
--strict treat validation warnings as errors
```
### Options inherited from parent commands