diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 3120a78..268c824 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -12,6 +12,5 @@ jobs: go-version: '1.22' - name: Build run: go build -v ./... -# TODO: Fix tests and enable step -# - name: Test -# run: go test -v ./... -tags integration + - name: Test + run: make test diff --git a/api/v1alpha1/releasemanfest_types_test.go b/api/v1alpha1/releasemanfest_types_test.go new file mode 100644 index 0000000..1efa367 --- /dev/null +++ b/api/v1alpha1/releasemanfest_types_test.go @@ -0,0 +1,29 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestArch_Short(t *testing.T) { + assert.Equal(t, "amd64", ArchTypeX86.Short()) + assert.Equal(t, "arm64", ArchTypeARM.Short()) + assert.PanicsWithValue(t, "unknown arch: abc", func() { + arch := Arch("abc") + arch.Short() + }) +} + +func TestSupportedArchitectures(t *testing.T) { + archs := []Arch{ArchTypeARM, ArchTypeX86} + + supported := SupportedArchitectures(archs) + require.Len(t, supported, 4) + + assert.Contains(t, supported, "x86_64") + assert.Contains(t, supported, "aarch64") + assert.Contains(t, supported, "amd64") + assert.Contains(t, supported, "arm64") +} diff --git a/api/v1alpha1/upgradeplan_webhook.go b/api/v1alpha1/upgradeplan_webhook.go index c9a4b7d..e6d2ec6 100644 --- a/api/v1alpha1/upgradeplan_webhook.go +++ b/api/v1alpha1/upgradeplan_webhook.go @@ -23,14 +23,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/version" ctrl "sigs.k8s.io/controller-runtime" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// log is for logging in this package. -var upgradeplanlog = logf.Log.WithName("upgradeplan-resource") - // SetupWebhookWithManager will setup the manager to manage the webhooks func (r *UpgradePlan) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). @@ -46,14 +42,12 @@ var _ webhook.Validator = &UpgradePlan{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type func (r *UpgradePlan) ValidateCreate() (admission.Warnings, error) { - upgradeplanlog.Info("validate create", "name", r.Name) - return nil, nil + _, err := validateReleaseVersion(r.Spec.ReleaseVersion) + return nil, err } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type func (r *UpgradePlan) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - upgradeplanlog.Info("validate update", "name", r.Name) - oldPlan, ok := old.(*UpgradePlan) if !ok { return nil, fmt.Errorf("unexpected object type: %T", old) @@ -65,12 +59,20 @@ func (r *UpgradePlan) ValidateUpdate(old runtime.Object) (admission.Warnings, er return nil, nil } - if oldPlan.Status.LastSuccessfulReleaseVersion != "" { - newReleaseVersion, err := version.ParseSemantic(r.Spec.ReleaseVersion) - if err != nil { - return nil, fmt.Errorf("'%s' is not a semantic version", r.Spec.ReleaseVersion) + disallowingUpdateStates := []string{UpgradeInProgress, UpgradePending, UpgradeError} + + for _, condition := range r.Status.Conditions { + if slices.Contains(disallowingUpdateStates, condition.Reason) { + return nil, fmt.Errorf("upgrade plan cannot be edited while condition '%s' is in '%s' state", condition.Type, condition.Reason) } + } + newReleaseVersion, err := validateReleaseVersion(r.Spec.ReleaseVersion) + if err != nil { + return nil, err + } + + if oldPlan.Status.LastSuccessfulReleaseVersion != "" { indicator, err := newReleaseVersion.Compare(oldPlan.Status.LastSuccessfulReleaseVersion) if err != nil { return nil, fmt.Errorf("comparing versions: %w", err) @@ -80,15 +82,7 @@ func (r *UpgradePlan) ValidateUpdate(old runtime.Object) (admission.Warnings, er case 0: return nil, fmt.Errorf("any edits over '%s' must come with an increment of the releaseVersion", r.Name) case -1: - return nil, fmt.Errorf("new releaseVersion '%s' must be greater than the currently applied '%s' releaseVersion", r.Spec.ReleaseVersion, oldPlan.Status.LastSuccessfulReleaseVersion) - } - } - - disallowingUpdateStates := []string{UpgradeInProgress, UpgradePending, UpgradeError} - - for _, condition := range r.Status.Conditions { - if slices.Contains(disallowingUpdateStates, condition.Reason) { - return nil, fmt.Errorf("upgrade plan cannot be edited while condition '%s' is in '%s' state", condition.Type, condition.Reason) + return nil, fmt.Errorf("new releaseVersion must be greater than the currently applied one ('%s')", oldPlan.Status.LastSuccessfulReleaseVersion) } } @@ -97,6 +91,18 @@ func (r *UpgradePlan) ValidateUpdate(old runtime.Object) (admission.Warnings, er // ValidateDelete implements webhook.Validator so a webhook will be registered for the type func (r *UpgradePlan) ValidateDelete() (admission.Warnings, error) { - upgradeplanlog.Info("validate delete", "name", r.Name) return nil, nil } + +func validateReleaseVersion(releaseVersion string) (*version.Version, error) { + if releaseVersion == "" { + return nil, fmt.Errorf("release version is required") + } + + v, err := version.ParseSemantic(releaseVersion) + if err != nil { + return nil, fmt.Errorf("'%s' is not a semantic version", releaseVersion) + } + + return v, nil +} diff --git a/api/v1alpha1/upgradeplan_webhook_test.go b/api/v1alpha1/upgradeplan_webhook_test.go index 29d75df..ff9f49b 100644 --- a/api/v1alpha1/upgradeplan_webhook_test.go +++ b/api/v1alpha1/upgradeplan_webhook_test.go @@ -18,21 +18,134 @@ package v1alpha1 import ( . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("UpgradePlan Webhook", func() { + Context("When creating UpgradePlans under Validating Webhook", func() { + It("Should be denied if release version is not specified", func() { + plan := &UpgradePlan{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plan1", + Namespace: "default", + }, + } + + err := k8sClient.Create(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("release version is required"))) + }) + + It("Should be denied if release version is not in semantic format", func() { + plan := &UpgradePlan{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plan1", + Namespace: "default", + }, + Spec: UpgradePlanSpec{ + ReleaseVersion: "v1", + }, + } - Context("When creating UpgradePlan under Validating Webhook", func() { - It("Should deny if a required field is empty", func() { + err := k8sClient.Create(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("'v1' is not a semantic version"))) + }) + }) - // TODO(user): Add your logic here + Context("When updating UpgradePlan under Validating Webhook", Ordered, func() { + plan := &UpgradePlan{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plan1", + Namespace: "default", + }, + Spec: UpgradePlanSpec{ + ReleaseVersion: "3.1.0", + }, + } + + BeforeAll(func() { + By("Creating the plan") + Expect(k8sClient.Create(ctx, plan)).To(Succeed()) + }) + AfterEach(func() { + By("Cleaning up status conditions") + plan.Status.Conditions = nil + Expect(k8sClient.Status().Update(ctx, plan)).To(Succeed()) }) - It("Should admit if all required fields are provided", func() { + It("Should be denied when an upgrade is pending", func() { + condition := metav1.Condition{Type: KubernetesUpgradedCondition, Status: metav1.ConditionFalse, Reason: UpgradePending} + + meta.SetStatusCondition(&plan.Status.Conditions, condition) + Expect(k8sClient.Status().Update(ctx, plan)).To(Succeed()) + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("upgrade plan cannot be edited while condition 'KubernetesUpgraded' is in 'Pending' state"))) + }) + + It("Should be denied when an upgrade is in progress", func() { + condition := metav1.Condition{Type: KubernetesUpgradedCondition, Status: metav1.ConditionFalse, Reason: UpgradeInProgress} + + meta.SetStatusCondition(&plan.Status.Conditions, condition) + Expect(k8sClient.Status().Update(ctx, plan)).To(Succeed()) + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("upgrade plan cannot be edited while condition 'KubernetesUpgraded' is in 'InProgress' state"))) + }) - // TODO(user): Add your logic here + It("Should be denied when an upgrade is experiencing a transient error", func() { + condition := metav1.Condition{Type: KubernetesUpgradedCondition, Status: metav1.ConditionFalse, Reason: UpgradeError} + + meta.SetStatusCondition(&plan.Status.Conditions, condition) + Expect(k8sClient.Status().Update(ctx, plan)).To(Succeed()) + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("upgrade plan cannot be edited while condition 'KubernetesUpgraded' is in 'Error' state"))) + }) + + It("Should be denied if release version is not specified", func() { + plan.Spec.ReleaseVersion = "" + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("release version is required"))) + }) + + It("Should be denied if release version is not in semantic format", func() { + plan.Spec.ReleaseVersion = "v1" + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("'v1' is not a semantic version"))) + }) + + It("Should be denied if the new release version is the same as the last applied one", func() { + plan.Status.LastSuccessfulReleaseVersion = "3.1.0" + Expect(k8sClient.Status().Update(ctx, plan)).To(Succeed()) + + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("any edits over 'plan1' must come with an increment of the releaseVersion"))) + }) + + It("Should be denied if the new release version is lesser than the last applied one", func() { + plan.Spec.ReleaseVersion = "3.0.2" + err := k8sClient.Update(ctx, plan) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("new releaseVersion must be greater than the currently applied one ('3.1.0')"))) + }) + It("Should pass if the new release version is greater than the last applied one", func() { + plan.Spec.ReleaseVersion = "3.1.1" + Expect(k8sClient.Update(ctx, plan)).To(Succeed()) }) })