diff --git a/cmd/create/kubeletconfig/cmd.go b/cmd/create/kubeletconfig/cmd.go index 10d9f25b06..bad2266c5f 100644 --- a/cmd/create/kubeletconfig/cmd.go +++ b/cmd/create/kubeletconfig/cmd.go @@ -88,36 +88,8 @@ func run(_ *cobra.Command, _ []string) { os.Exit(1) } - if args.podPidsLimit == PodPidsLimitOptionDefaultValue && !interactive.Enabled() { - interactive.Enable() - r.Reporter.Infof("Enabling interactive mode") - } - - if interactive.Enabled() { - args.podPidsLimit, err = interactive.GetInt(interactive.Input{ - Question: InteractivePodPidsLimitPrompt, - Help: InteractivePodPidsLimitHelp, - Options: nil, - Default: nil, - Required: true, - Validators: []interactive.Validator{ - // We only validate the minimum as some customers have capability to exceed maximum - // We rely on backend validation for that - interactive.Min(MinPodPidsLimit), - }, - }) - - if err != nil { - r.Reporter.Errorf("Failed creating KubeletConfig for cluster '%s': '%s'", - clusterKey, err) - os.Exit(1) - } - - } - - if args.podPidsLimit < MinPodPidsLimit { - r.Reporter.Errorf("The minimum value for --pod-pids-limit is '%d'. You have supplied '%d'", - MinPodPidsLimit, args.podPidsLimit) + requestedPids, err := ValidateOrPromptForRequestedPidsLimit(args.podPidsLimit, clusterKey, nil, r) + if err != nil { os.Exit(1) } @@ -127,7 +99,7 @@ func run(_ *cobra.Command, _ []string) { if confirm.ConfirmRaw(prompt) { r.Reporter.Debugf("Creating KubeletConfig for cluster '%s'", clusterKey) - kubeletConfigArgs := ocm.KubeletConfigArgs{PodPidsLimit: args.podPidsLimit} + kubeletConfigArgs := ocm.KubeletConfigArgs{PodPidsLimit: requestedPids} _, err = r.OCMClient.CreateKubeletConfig(cluster.ID(), kubeletConfigArgs) if err != nil { diff --git a/cmd/edit/kubeletconfig/cmd.go b/cmd/edit/kubeletconfig/cmd.go index 29cd0db9cf..4125ba8807 100644 --- a/cmd/edit/kubeletconfig/cmd.go +++ b/cmd/edit/kubeletconfig/cmd.go @@ -92,35 +92,8 @@ func run(_ *cobra.Command, _ []string) { r.Reporter.Debugf("Updating KubeletConfig for cluster '%s'", clusterKey) - if args.podPidsLimit == PodPidsLimitOptionDefaultValue && !interactive.Enabled() { - interactive.Enable() - r.Reporter.Infof("Enabling interactive mode") - } - - if interactive.Enabled() { - args.podPidsLimit, err = interactive.GetInt(interactive.Input{ - Question: InteractivePodPidsLimitPrompt, - Help: InteractivePodPidsLimitHelp, - Options: nil, - Default: kubeletconfig.PodPidsLimit(), - Required: true, - Validators: []interactive.Validator{ - // We only validate the minimum as some customers have capability to exceed maximum - // We rely on backend validation for that - interactive.Min(MinPodPidsLimit), - }, - }) - - if err != nil { - r.Reporter.Errorf("Failed updating KubeletConfig for cluster '%s': '%s'", - clusterKey, err) - os.Exit(1) - } - } - - if args.podPidsLimit < MinPodPidsLimit { - r.Reporter.Errorf("The minimum value for --pod-pids-limit is '%d'. You have supplied '%d'", - MinPodPidsLimit, args.podPidsLimit) + requestedPids, err := ValidateOrPromptForRequestedPidsLimit(args.podPidsLimit, clusterKey, kubeletconfig, r) + if err != nil { os.Exit(1) } @@ -129,7 +102,7 @@ func run(_ *cobra.Command, _ []string) { if confirm.ConfirmRaw(prompt) { r.Reporter.Debugf("Updating KubeletConfig for cluster '%s'", clusterKey) - _, err = r.OCMClient.UpdateKubeletConfig(cluster.ID(), ocm.KubeletConfigArgs{PodPidsLimit: args.podPidsLimit}) + _, err = r.OCMClient.UpdateKubeletConfig(cluster.ID(), ocm.KubeletConfigArgs{PodPidsLimit: requestedPids}) if err != nil { r.Reporter.Errorf("Failed creating custom KubeletConfig for cluster '%s': %s", cluster.ID(), err) diff --git a/pkg/interactive/validation.go b/pkg/interactive/validation.go index 79aa797ea1..72f6fd319f 100644 --- a/pkg/interactive/validation.go +++ b/pkg/interactive/validation.go @@ -124,12 +124,33 @@ func IsCIDR(val interface{}) error { return fmt.Errorf("can only validate strings, got %v", val) } -func Min(min int) Validator { +// MaxValue returns a Validator that validates the entered number is less than or equal to max +func MaxValue(max int) Validator { return func(ans interface{}) error { - if i, ok := ans.(string); ok { - val, err := strconv.Atoi(i) + if str, ok := ans.(string); ok { + val, err := strconv.Atoi(str) if err != nil { - return fmt.Errorf("please enter an integer value, you entered '%s'", i) + return fmt.Errorf("please enter an integer value, you entered '%s'", str) + } + if val > max { + return fmt.Errorf( + "'%d' is greater than the permitted maximum of '%d'", val, max) + } + + return nil + } + + return fmt.Errorf("can only validate strings, got %v", ans) + } +} + +// MinValue returns a validator that validates the entered number is greater than or equal to min +func MinValue(min int) Validator { + return func(ans interface{}) error { + if str, ok := ans.(string); ok { + val, err := strconv.Atoi(str) + if err != nil { + return fmt.Errorf("please enter an integer value, you entered '%s'", str) } if val < min { return fmt.Errorf( diff --git a/pkg/interactive/validation_test.go b/pkg/interactive/validation_test.go index c089a19516..73750aed30 100644 --- a/pkg/interactive/validation_test.go +++ b/pkg/interactive/validation_test.go @@ -6,9 +6,9 @@ import ( ) var _ = Describe("Validation", func() { - Context("Min", func() { + Context("MinValue", func() { It("Fails validation if the answer is less than the minimum", func() { - validator := Min(50) + validator := MinValue(50) err := validator("25") Expect(err).To(HaveOccurred()) @@ -16,29 +16,66 @@ var _ = Describe("Validation", func() { }) It("Fails validation if the answer is not an integer", func() { - validator := Min(50) + validator := MinValue(50) err := validator("hello") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("please enter an integer value, you entered 'hello'")) }) It("Fails validation if the answer is not a string", func() { - validator := Min(50) + validator := MinValue(50) err := validator(45) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("can only validate strings, got int")) }) It("Passes validation if the answer is greater than the min", func() { - validator := Min(50) + validator := MinValue(50) err := validator("55") Expect(err).NotTo(HaveOccurred()) }) It("Passes validation if the answer is equal to the min", func() { - validator := Min(50) + validator := MinValue(50) err := validator("50") Expect(err).NotTo(HaveOccurred()) }) }) + + Context("MaxValue", func() { + It("Fails validation if the answer is greater than the maximum", func() { + validator := MaxValue(50) + err := validator("52") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("'52' is greater than the permitted maximum of '50'")) + }) + + It("Fails validation if the answer is not an integer", func() { + validator := MaxValue(50) + err := validator("hello") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("please enter an integer value, you entered 'hello'")) + }) + + It("Fails validation if the answer is not a string", func() { + validator := MaxValue(50) + err := validator(45) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("can only validate strings, got int")) + }) + + It("Passes validation if the answer is less than the max", func() { + validator := MaxValue(50) + err := validator("49") + Expect(err).NotTo(HaveOccurred()) + }) + + It("Passes validation if the answer is equal to the max", func() { + validator := MaxValue(50) + err := validator("50") + Expect(err).NotTo(HaveOccurred()) + }) + }) + }) diff --git a/pkg/kubeletconfig/config.go b/pkg/kubeletconfig/config.go new file mode 100644 index 0000000000..08f73e5183 --- /dev/null +++ b/pkg/kubeletconfig/config.go @@ -0,0 +1,98 @@ +package kubeletconfig + +import ( + "fmt" + v1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" + "github.com/openshift/rosa/pkg/rosa" + + "github.com/openshift/rosa/pkg/interactive" +) + +//go:generate mockgen -source=config.go -package=kubeletconfig -destination=mock_capability_checker.go +type CapabilityChecker interface { + IsCapabilityEnabled(capability string) (bool, error) +} + +// GetMaxPidsLimit - returns the maximum pids limit for the current organization +// the maximum is varied depending on whether the current organizaton has +// the capability.organization.bypass_pids_limit capability +func GetMaxPidsLimit(client CapabilityChecker) (int, error) { + enabled, err := client.IsCapabilityEnabled(ByPassPidsLimitCapability) + if err != nil { + return -1, err + } + + if enabled { + return MaxUnsafePodPidsLimit, nil + } + return MaxPodPidsLimit, nil +} + +func GetInteractiveMaxPidsLimitHelp(maxPidsLimit int) string { + return fmt.Sprintf(InteractivePodPidsLimitHelp, maxPidsLimit) +} + +func GetInteractiveInput(maxPidsLimit int, kubeletConfig *v1.KubeletConfig) interactive.Input { + + var defaultLimit = PodPidsLimitOptionDefaultValue + if kubeletConfig != nil { + defaultLimit = kubeletConfig.PodPidsLimit() + } + + return interactive.Input{ + Question: InteractivePodPidsLimitPrompt, + Help: GetInteractiveMaxPidsLimitHelp(maxPidsLimit), + Options: nil, + Default: defaultLimit, + Required: true, + Validators: []interactive.Validator{ + interactive.MinValue(MinPodPidsLimit), + interactive.MaxValue(maxPidsLimit), + }, + } +} + +// ValidateOrPromptForRequestedPidsLimit validates user provided limits or prompts via interactive mode +// if the user hasn't specified any limit on the command line. +func ValidateOrPromptForRequestedPidsLimit( + requestedPids int, + clusterKey string, + kubeletConfig *v1.KubeletConfig, + r *rosa.Runtime) (int, error) { + + if requestedPids == PodPidsLimitOptionDefaultValue && !interactive.Enabled() { + interactive.Enable() + r.Reporter.Infof("Enabling interactive mode") + } + + maxPidsLimit, err := GetMaxPidsLimit(r.OCMClient) + if err != nil { + return PodPidsLimitOptionDefaultValue, + r.Reporter.Errorf("Failed to check maximum allowed Pids limit for cluster '%s'", + clusterKey) + } + + if interactive.Enabled() { + requestedPids, err = interactive.GetInt(GetInteractiveInput(maxPidsLimit, kubeletConfig)) + + if err != nil { + return PodPidsLimitOptionDefaultValue, + r.Reporter.Errorf("Failed reading requested Pids limit for cluster '%s': '%s'", + clusterKey, err) + } + } + + if requestedPids < MinPodPidsLimit { + return PodPidsLimitOptionDefaultValue, + r.Reporter.Errorf("The minimum value for --pod-pids-limit is '%d'. You have supplied '%d'", + MinPodPidsLimit, requestedPids) + } + + if requestedPids > maxPidsLimit { + return PodPidsLimitOptionDefaultValue, + r.Reporter.Errorf("The maximum value for --pod-pids-limit is '%d'. You have supplied '%d'", + maxPidsLimit, requestedPids) + } + + return requestedPids, nil +} diff --git a/pkg/kubeletconfig/config_test.go b/pkg/kubeletconfig/config_test.go new file mode 100644 index 0000000000..b005523bb5 --- /dev/null +++ b/pkg/kubeletconfig/config_test.go @@ -0,0 +1,71 @@ +package kubeletconfig + +import ( + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2/dsl/core" + . "github.com/onsi/gomega" + v1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" +) + +var _ = Describe("KubeletConfig Config", func() { + + Context("GetMaxPidsLimit", func() { + + var ctrl *gomock.Controller + var capabilityChecker *MockCapabilityChecker + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + capabilityChecker = NewMockCapabilityChecker(ctrl) + }) + + It("Returns Correct Max Pids Limit When Org Has Capability", func() { + capabilityChecker.EXPECT().IsCapabilityEnabled(ByPassPidsLimitCapability).Return(true, nil) + + max, err := GetMaxPidsLimit(capabilityChecker) + Expect(err).NotTo(HaveOccurred()) + Expect(max).To(Equal(MaxUnsafePodPidsLimit)) + }) + + It("Returns Correct Max Pids Limit When Org Does Not Have Capability", func() { + capabilityChecker.EXPECT().IsCapabilityEnabled(ByPassPidsLimitCapability).Return(false, nil) + + max, err := GetMaxPidsLimit(capabilityChecker) + Expect(err).NotTo(HaveOccurred()) + Expect(max).To(Equal(MaxPodPidsLimit)) + }) + }) + + Context("GetInteractiveMaxPidsLimitHelp", func() { + It("Correctly generates the Max Pids Limit Interactive Help", func() { + help := GetInteractiveMaxPidsLimitHelp(5000) + Expect(help).To(Equal("Set the Pod Pids Limit field to a value between 4096 and 5000")) + }) + }) + + Context("GetInteractiveInput", func() { + It("Correctly generates Interactive Input for pre-existing KubeletConfig", func() { + + builder := v1.KubeletConfigBuilder{} + kubeletConfig, err := builder.PodPidsLimit(10000).Build() + + Expect(err).NotTo(HaveOccurred()) + + input := GetInteractiveInput(5000, kubeletConfig) + Expect(input.Required).To(BeTrue()) + Expect(input.Question).To(Equal(InteractivePodPidsLimitPrompt)) + Expect(input.Help).To(Equal(GetInteractiveMaxPidsLimitHelp(5000))) + Expect(len(input.Validators)).To(Equal(2)) + Expect(input.Default).To(Equal(kubeletConfig.PodPidsLimit())) + }) + + It("Correctly generates Interactive Input for new KubeletConfig", func() { + input := GetInteractiveInput(5000, nil) + Expect(input.Required).To(BeTrue()) + Expect(input.Question).To(Equal(InteractivePodPidsLimitPrompt)) + Expect(input.Help).To(Equal(GetInteractiveMaxPidsLimitHelp(5000))) + Expect(len(input.Validators)).To(Equal(2)) + Expect(input.Default).To(Equal(PodPidsLimitOptionDefaultValue)) + }) + }) +}) diff --git a/pkg/kubeletconfig/consts.go b/pkg/kubeletconfig/consts.go index 2994712fd5..7e06506fb4 100644 --- a/pkg/kubeletconfig/consts.go +++ b/pkg/kubeletconfig/consts.go @@ -1,12 +1,13 @@ package kubeletconfig const ( - MinPodPidsLimit = 4096 - PodPidsLimitOption = "pod-pids-limit" - PodPidsLimitOptionUsage = "Sets the requested pod_pids_limit for your custom KubeletConfig." + - " Must be an integer in the range 4096 - 16,384." + MinPodPidsLimit = 4096 + MaxPodPidsLimit = 16384 + MaxUnsafePodPidsLimit = 3694303 + PodPidsLimitOption = "pod-pids-limit" + PodPidsLimitOptionUsage = "Sets the requested pod_pids_limit for your custom KubeletConfig." PodPidsLimitOptionDefaultValue = -1 - - InteractivePodPidsLimitPrompt = "Pod Pids Limit?" - InteractivePodPidsLimitHelp = "Set the Pod Pids Limit field to a value between 4096 and 16,384" + InteractivePodPidsLimitPrompt = "Pod Pids Limit?" + InteractivePodPidsLimitHelp = "Set the Pod Pids Limit field to a value between 4096 and %d" + ByPassPidsLimitCapability = "capability.organization.bypass_pids_limits" ) diff --git a/pkg/kubeletconfig/kubeletconfig_test.go b/pkg/kubeletconfig/kubeletconfig_test.go new file mode 100644 index 0000000000..9b428a856c --- /dev/null +++ b/pkg/kubeletconfig/kubeletconfig_test.go @@ -0,0 +1,13 @@ +package kubeletconfig + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestKubeletConfig(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Ocm Suite") +} diff --git a/pkg/kubeletconfig/mock_capability_checker.go b/pkg/kubeletconfig/mock_capability_checker.go new file mode 100644 index 0000000000..c200fb8038 --- /dev/null +++ b/pkg/kubeletconfig/mock_capability_checker.go @@ -0,0 +1,49 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: config.go + +// Package kubeletconfig is a generated GoMock package. +package kubeletconfig + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockCapabilityChecker is a mock of CapabilityChecker interface. +type MockCapabilityChecker struct { + ctrl *gomock.Controller + recorder *MockCapabilityCheckerMockRecorder +} + +// MockCapabilityCheckerMockRecorder is the mock recorder for MockCapabilityChecker. +type MockCapabilityCheckerMockRecorder struct { + mock *MockCapabilityChecker +} + +// NewMockCapabilityChecker creates a new mock instance. +func NewMockCapabilityChecker(ctrl *gomock.Controller) *MockCapabilityChecker { + mock := &MockCapabilityChecker{ctrl: ctrl} + mock.recorder = &MockCapabilityCheckerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockCapabilityChecker) EXPECT() *MockCapabilityCheckerMockRecorder { + return m.recorder +} + +// IsCapabilityEnabled mocks base method. +func (m *MockCapabilityChecker) IsCapabilityEnabled(capability string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsCapabilityEnabled", capability) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsCapabilityEnabled indicates an expected call of IsCapabilityEnabled. +func (mr *MockCapabilityCheckerMockRecorder) IsCapabilityEnabled(capability interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsCapabilityEnabled", reflect.TypeOf((*MockCapabilityChecker)(nil).IsCapabilityEnabled), capability) +}