From dd196d3006d29ae5cae5d43b648da1ca2e5af236 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 28 Oct 2021 20:43:54 +0300 Subject: [PATCH] refactor: prepare for move of pkg/resources to machinery Start enforcing importvet rules, the very first cleanup is breaking dependency of `pkg/resources` on `pkg/kernel` (only machinery imports are allowed). Follow-up PRs will address other problematic imports. Signed-off-by: Andrey Smirnov --- Makefile | 2 +- .../runtime/kernel_param_defaults.go | 2 +- .../runtime/kernel_param_defaults_test.go | 2 +- .../controllers/runtime/kernel_param_spec.go | 11 +++--- .../runtime/kernel_param_spec_test.go | 7 ++-- .../v1alpha1/v1alpha1_sequencer_tasks.go | 7 ++-- pkg/kernel/kernel.go | 21 +++-------- pkg/kernel/kspp/kspp.go | 2 +- pkg/machinery/kernel/kernel.go | 16 ++++++++ pkg/resources/.importvet.yaml | 23 ++++++++++++ pkg/resources/runtime/condition.go | 2 +- pkg/resources/runtime/condition_test.go | 37 ++++++++++++++++--- 12 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 pkg/resources/.importvet.yaml diff --git a/Makefile b/Makefile index c28f5c078..524084f7f 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ GOFUMPT_VERSION ?= v0.1.0 STRINGER_VERSION ?= v0.1.5 DEEPCOPY_GEN_VERSION ?= v0.21.3 VTPROTOBUF_VERSION ?= 81d623a9a700ede8ef765e5ab08b3aa1f5b4d5a8 -IMPORTVET ?= autonomy/importvet:f6b07d9 +IMPORTVET ?= ghcr.io/talos-systems/importvet:c9424fe OPERATING_SYSTEM := $(shell uname -s | tr "[:upper:]" "[:lower:]") TALOSCTL_DEFAULT_TARGET := talosctl-$(OPERATING_SYSTEM) INTEGRATION_TEST_DEFAULT_TARGET := integration-test-$(OPERATING_SYSTEM) diff --git a/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults.go b/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults.go index 18e6f6afb..45cf187ef 100644 --- a/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults.go +++ b/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults.go @@ -12,8 +12,8 @@ import ( "go.uber.org/zap" v1alpha1runtime "github.com/talos-systems/talos/internal/app/machined/pkg/runtime" - "github.com/talos-systems/talos/pkg/kernel" "github.com/talos-systems/talos/pkg/kernel/kspp" + "github.com/talos-systems/talos/pkg/machinery/kernel" "github.com/talos-systems/talos/pkg/resources/runtime" ) diff --git a/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults_test.go b/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults_test.go index aa7a10ff2..8b0cc259c 100644 --- a/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults_test.go +++ b/internal/app/machined/pkg/controllers/runtime/kernel_param_defaults_test.go @@ -14,7 +14,7 @@ import ( runtimecontrollers "github.com/talos-systems/talos/internal/app/machined/pkg/controllers/runtime" "github.com/talos-systems/talos/internal/app/machined/pkg/runtime" - "github.com/talos-systems/talos/pkg/kernel" + "github.com/talos-systems/talos/pkg/machinery/kernel" runtimeresource "github.com/talos-systems/talos/pkg/resources/runtime" ) diff --git a/internal/app/machined/pkg/controllers/runtime/kernel_param_spec.go b/internal/app/machined/pkg/controllers/runtime/kernel_param_spec.go index a64c11053..e4c896404 100644 --- a/internal/app/machined/pkg/controllers/runtime/kernel_param_spec.go +++ b/internal/app/machined/pkg/controllers/runtime/kernel_param_spec.go @@ -15,7 +15,8 @@ import ( "github.com/hashicorp/go-multierror" "go.uber.org/zap" - "github.com/talos-systems/talos/pkg/kernel" + krnl "github.com/talos-systems/talos/pkg/kernel" + "github.com/talos-systems/talos/pkg/machinery/kernel" "github.com/talos-systems/talos/pkg/resources/runtime" ) @@ -122,14 +123,14 @@ func (ctrl *KernelParamSpecController) updateKernelParam(ctx context.Context, r prop := &kernel.Param{Key: key, Value: value} if _, ok := ctrl.defaults[key]; !ok { - if data, err := kernel.ReadParam(prop); err == nil { + if data, err := krnl.ReadParam(prop); err == nil { ctrl.defaults[key] = string(data) } else if !errors.Is(err, os.ErrNotExist) { return err } } - if err := kernel.WriteParam(prop); err != nil { + if err := krnl.WriteParam(prop); err != nil { return err } @@ -149,12 +150,12 @@ func (ctrl *KernelParamSpecController) resetKernelParam(ctx context.Context, r c var err error if def, ok := ctrl.defaults[key]; ok { - err = kernel.WriteParam(&kernel.Param{ + err = krnl.WriteParam(&kernel.Param{ Key: key, Value: def, }) } else { - err = kernel.DeleteParam(&kernel.Param{Key: key}) + err = krnl.DeleteParam(&kernel.Param{Key: key}) } if err != nil { diff --git a/internal/app/machined/pkg/controllers/runtime/kernel_param_spec_test.go b/internal/app/machined/pkg/controllers/runtime/kernel_param_spec_test.go index b468c40b5..1be938649 100644 --- a/internal/app/machined/pkg/controllers/runtime/kernel_param_spec_test.go +++ b/internal/app/machined/pkg/controllers/runtime/kernel_param_spec_test.go @@ -16,7 +16,8 @@ import ( "github.com/talos-systems/go-retry/retry" runtimecontrollers "github.com/talos-systems/talos/internal/app/machined/pkg/controllers/runtime" - "github.com/talos-systems/talos/pkg/kernel" + krnl "github.com/talos-systems/talos/pkg/kernel" + "github.com/talos-systems/talos/pkg/machinery/kernel" runtimeresource "github.com/talos-systems/talos/pkg/resources/runtime" ) @@ -50,7 +51,7 @@ func (suite *KernelParamSpecSuite) TestParamsSynced() { ), )) - prop, err := kernel.ReadParam(&kernel.Param{Key: fsFileMax}) + prop, err := krnl.ReadParam(&kernel.Param{Key: fsFileMax}) suite.Assert().NoError(err) suite.Require().Equal(value, strings.TrimSpace(string(prop))) @@ -74,7 +75,7 @@ func (suite *KernelParamSpecSuite) TestParamsSynced() { }, )) - prop, err = kernel.ReadParam(&kernel.Param{Key: fsFileMax}) + prop, err = krnl.ReadParam(&kernel.Param{Key: fsFileMax}) suite.Assert().NoError(err) suite.Require().Equal(def, strings.TrimSpace(string(prop))) } diff --git a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go index 8484489a0..0ba42edf2 100644 --- a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go +++ b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go @@ -57,7 +57,7 @@ import ( "github.com/talos-systems/talos/internal/pkg/partition" "github.com/talos-systems/talos/pkg/conditions" "github.com/talos-systems/talos/pkg/images" - "github.com/talos-systems/talos/pkg/kernel" + krnl "github.com/talos-systems/talos/pkg/kernel" "github.com/talos-systems/talos/pkg/kernel/kspp" "github.com/talos-systems/talos/pkg/kubernetes" machineapi "github.com/talos-systems/talos/pkg/machinery/api/machine" @@ -66,6 +66,7 @@ import ( "github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1" "github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1/machine" "github.com/talos-systems/talos/pkg/machinery/constants" + "github.com/talos-systems/talos/pkg/machinery/kernel" resourceruntime "github.com/talos-systems/talos/pkg/resources/runtime" "github.com/talos-systems/talos/pkg/version" ) @@ -88,7 +89,7 @@ func SetupLogger(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionF // disable ratelimiting for kmsg, otherwise logs might be not visible. // this should be set via kernel arg, but in case it's not set, try to force it. - if err = kernel.WriteParam(&kernel.Param{ + if err = krnl.WriteParam(&kernel.Param{ Key: "kernel.printk_devkmsg", Value: "on\n", }); err != nil { @@ -241,7 +242,7 @@ func SetRLimit(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFun // DropCapabilities drops some capabilities so that they can't be restored by child processes. func DropCapabilities(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, string) { return func(ctx context.Context, logger *log.Logger, r runtime.Runtime) error { - prop, err := kernel.ReadParam(&kernel.Param{Key: "kernel.kexec_load_disabled"}) + prop, err := krnl.ReadParam(&kernel.Param{Key: "kernel.kexec_load_disabled"}) if v := strings.TrimSpace(string(prop)); err == nil && v != "0" { logger.Printf("kernel.kexec_load_disabled is %v, skipping dropping capabilities", v) diff --git a/pkg/kernel/kernel.go b/pkg/kernel/kernel.go index e4208ecdc..63a2c89b8 100644 --- a/pkg/kernel/kernel.go +++ b/pkg/kernel/kernel.go @@ -7,32 +7,21 @@ package kernel import ( "io/ioutil" "os" - "path" - "strings" + + "github.com/talos-systems/talos/pkg/machinery/kernel" ) -// Param represents a kernel system property. -type Param struct { - Key string - Value string -} - // WriteParam writes a value to a key under /proc/sys. -func WriteParam(prop *Param) error { +func WriteParam(prop *kernel.Param) error { return ioutil.WriteFile(prop.Path(), []byte(prop.Value), 0o644) } // ReadParam reads a value from a key under /proc/sys. -func ReadParam(prop *Param) ([]byte, error) { +func ReadParam(prop *kernel.Param) ([]byte, error) { return ioutil.ReadFile(prop.Path()) } // DeleteParam deletes a value from a key under /proc/sys. -func DeleteParam(prop *Param) error { +func DeleteParam(prop *kernel.Param) error { return os.Remove(prop.Path()) } - -// Path returns the path to the systctl file under /proc/sys. -func (prop *Param) Path() string { - return path.Join("/proc/sys", strings.ReplaceAll(prop.Key, ".", "/")) -} diff --git a/pkg/kernel/kspp/kspp.go b/pkg/kernel/kspp/kspp.go index b61785c66..47a02c8b6 100644 --- a/pkg/kernel/kspp/kspp.go +++ b/pkg/kernel/kspp/kspp.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/talos-systems/go-procfs/procfs" - "github.com/talos-systems/talos/pkg/kernel" + "github.com/talos-systems/talos/pkg/machinery/kernel" ) // RequiredKSPPKernelParameters is the set of kernel parameters required to diff --git a/pkg/machinery/kernel/kernel.go b/pkg/machinery/kernel/kernel.go index c93ff19bd..4a5c99a9e 100644 --- a/pkg/machinery/kernel/kernel.go +++ b/pkg/machinery/kernel/kernel.go @@ -4,6 +4,11 @@ package kernel +import ( + "path" + "strings" +) + // DefaultArgs returns the Talos default kernel commandline options. var DefaultArgs = []string{ "init_on_alloc=1", @@ -20,3 +25,14 @@ var DefaultArgs = []string{ "ima_appraise=fix", "ima_hash=sha512", } + +// Param represents a kernel system property. +type Param struct { + Key string + Value string +} + +// Path returns the path to the systctl file under /proc/sys. +func (prop *Param) Path() string { + return path.Join("/proc/sys", strings.ReplaceAll(prop.Key, ".", "/")) +} diff --git a/pkg/resources/.importvet.yaml b/pkg/resources/.importvet.yaml new file mode 100644 index 000000000..9c8e73f5e --- /dev/null +++ b/pkg/resources/.importvet.yaml @@ -0,0 +1,23 @@ +--- +# temporary rules to facilitate moving `pkg/resources` into `pkg/machinery`: +# - no imports of anything from Talos except for machinery and pkg/resources itself +# - (not enforced yet) external dependencies we don't have to see in the machinery +rules: + - regexp: ^github.com/talos-systems/talos + action: deny + - regexp: ^github.com/talos-systems/talos/pkg/resources + action: allow + - regexp: ^github.com/talos-systems/talos/pkg/machinery + action: allow + #- regexp: ^k8s.io/ + # action: deny + #- regexp: ^github.com/jxskiss/base62 + # action: deny + #- regexp: ^github.com/mdlayher/netlink + # action: deny + #- regexp: ^github.com/mdlayher/netx + # action: deny + #- regexp: ^github.com/prometheus/procfs + # action: deny + #- regexp: ^golang.zx2c4.com/wireguard/wgctrl + # action: deny diff --git a/pkg/resources/runtime/condition.go b/pkg/resources/runtime/condition.go index b72cf603f..b828eb23c 100644 --- a/pkg/resources/runtime/condition.go +++ b/pkg/resources/runtime/condition.go @@ -10,7 +10,7 @@ import ( "github.com/cosi-project/runtime/pkg/resource" "github.com/cosi-project/runtime/pkg/state" - "github.com/talos-systems/talos/pkg/kernel" + "github.com/talos-systems/talos/pkg/machinery/kernel" ) // KernelParamsSetCondition implements condition which waits for the kernels to be in sync. diff --git a/pkg/resources/runtime/condition_test.go b/pkg/resources/runtime/condition_test.go index 5bb8ecd6a..ec3624776 100644 --- a/pkg/resources/runtime/condition_test.go +++ b/pkg/resources/runtime/condition_test.go @@ -16,8 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/talos-systems/talos/pkg/kernel" - "github.com/talos-systems/talos/pkg/kernel/kspp" + "github.com/talos-systems/talos/pkg/machinery/kernel" "github.com/talos-systems/talos/pkg/resources/runtime" ) @@ -77,10 +76,36 @@ func TestCondition(t *testing.T) { Succeeds: false, }, { - Name: "multiple values", - ActualKernelParams: kspp.GetKernelParams(), - AwaitKernelParams: kspp.GetKernelParams(), - Succeeds: true, + Name: "multiple values", + ActualKernelParams: []*kernel.Param{ + { + Key: "kernel.kptr_restrict", + Value: "1", + }, + { + Key: "kernel.dmesg_restrict", + Value: "1", + }, + { + Key: "kernel.perf_event_paranoid", + Value: "3", + }, + }, + AwaitKernelParams: []*kernel.Param{ + { + Key: "kernel.kptr_restrict", + Value: "1", + }, + { + Key: "kernel.dmesg_restrict", + Value: "1", + }, + { + Key: "kernel.perf_event_paranoid", + Value: "3", + }, + }, + Succeeds: true, }, } { tt := tt