diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml new file mode 100644 index 0000000..63b3e45 --- /dev/null +++ b/.github/workflows/codeql.yaml @@ -0,0 +1,51 @@ +name: CodeQL +on: + push: + branches: + - 'main' + pull_request: + branches: + - 'main' + schedule: + - cron: '21 3 * * 3' + +permissions: read-all + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + security-events: write + packages: read + actions: read + contents: read + + strategy: + fail-fast: false + matrix: + language: ['go'] + + steps: + - name: checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Set up Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version: 1.26.5 + + - name: Initialize CodeQL + uses: github/codeql-action/init@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 + with: + languages: ${{ matrix.language }} + + - name: Build + # Autobuild's heuristic only traced a fraction of the module (79 of 189 files per the + # code-scanning diagnostic). Build the whole module explicitly so CodeQL sees every package. + run: go build ./... + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/scorecard.yaml b/.github/workflows/scorecard.yaml new file mode 100644 index 0000000..946c823 --- /dev/null +++ b/.github/workflows/scorecard.yaml @@ -0,0 +1,45 @@ +name: Scorecard supply-chain security +on: + branch_protection_rule: + schedule: + - cron: '30 2 * * 1' + push: + branches: + - 'main' + +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-latest + permissions: + security-events: write + id-token: write + contents: read + actions: read + + steps: + - name: checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Run analysis + uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3 + with: + results_file: results.sarif + results_format: sarif + publish_results: true + + - name: Upload artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + - name: Upload to code-scanning + uses: github/codeql-action/upload-sarif@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 + with: + sarif_file: results.sarif diff --git a/README.md b/README.md index cd426f3..35e3522 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ [![CI](https://github.com/projectsveltos/libsveltos/actions/workflows/main.yaml/badge.svg)](https://github.com/projectsveltos/libsveltos/actions) -[![Go Report Card](https://goreportcard.com/badge/github.com/projectsveltos/libsveltos)](https://goreportcard.com/report/github.com/projectsveltos/libsveltos) +[![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/projectsveltos/libsveltos/badge)](https://scorecard.dev/viewer/?uri=github.com/projectsveltos/libsveltos) +[![CodeQL](https://github.com/projectsveltos/libsveltos/actions/workflows/codeql.yaml/badge.svg)](https://github.com/projectsveltos/libsveltos/actions/workflows/codeql.yaml) [![Release](https://img.shields.io/github/v/release/projectsveltos/libsveltos)](https://github.com/projectsveltos/libsveltos/releases) [![License](https://img.shields.io/badge/license-Apache-blue.svg)](LICENSE) [![Slack](https://img.shields.io/badge/join%20slack-%23projectsveltos-brighteen)](https://join.slack.com/t/projectsveltos/shared_invite/zt-1hraownbr-W8NTs6LTimxLPB8Erj8Q6Q) @@ -22,7 +23,7 @@ ## What this repository is Libraries containing all pieces common across different repositories -## Contributing +## Contributing ❤️ Your contributions are always welcome! If you want to contribute, have questions, noticed any bug or want to get the latest project news, you can connect with us in the following ways: diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..1e24d31 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,47 @@ +# Security Policy + +## Supported Versions + +We release security fixes for the latest minor version. We encourage all users to stay on the latest release. + +| Version | Supported | +|----------------|--------------------| +| latest release | :white_check_mark: | +| older releases | :x: | + +## Reporting a Vulnerability + +**Please do not report security vulnerabilities through public GitHub issues.** + +If you believe you have found a security vulnerability in any Sveltos repository, please report it responsibly by sending an email to: + +**support@projectsveltos.io** + +Please include as much of the following information as possible to help us understand and resolve the issue quickly: + +- A description of the vulnerability and its potential impact +- The affected component(s) and version(s) +- Step-by-step instructions to reproduce the issue +- Any proof-of-concept or exploit code (if applicable) +- Suggested remediation (if any) + +## Response Process + +- You will receive an acknowledgement within **2 business days** +- We will investigate and keep you informed of our progress +- Once the issue is confirmed, we will work on a fix and coordinate a release +- We will publicly disclose the vulnerability after a fix is available, giving you credit unless you prefer to remain anonymous + +## Scope + +This policy covers all projects under the [projectsveltos](https://github.com/projectsveltos) GitHub organization. + +## Out of Scope + +- Vulnerabilities in dependencies (please report those to the upstream project) +- Issues in non-production branches or unreleased code +- Social engineering attacks + +## Thank You + +We appreciate responsible disclosure and the work of the security community in keeping Sveltos and its users safe. diff --git a/api/v1beta1/configurationbundle_type.go b/api/v1beta1/configurationbundle_type.go index bf1c661..9a43f7a 100644 --- a/api/v1beta1/configurationbundle_type.go +++ b/api/v1beta1/configurationbundle_type.go @@ -112,6 +112,14 @@ type ConfigurationBundleSpec struct { // +kubebuilder:default:=false // +optional SkipNamespaceCreation bool `json:"skipNamespaceCreation,omitempty"` + + // Force indicates whether Sveltos should delete and recreate a resource defined in this + // PolicyRef/KustomizationRef when an update is rejected with an error that only a + // delete+recreate can resolve (eg an invalid combination of fields, or a field enforced + // as immutable). By default, such errors are surfaced instead of recreating the resource. + // +kubebuilder:default:=false + // +optional + Force bool `json:"force,omitempty"` } type ConfigurationBundleStatus struct { diff --git a/config/crd/bases/lib.projectsveltos.io_configurationbundles.yaml b/config/crd/bases/lib.projectsveltos.io_configurationbundles.yaml index c960066..ef96cc0 100644 --- a/config/crd/bases/lib.projectsveltos.io_configurationbundles.yaml +++ b/config/crd/bases/lib.projectsveltos.io_configurationbundles.yaml @@ -39,6 +39,14 @@ spec: type: object spec: properties: + force: + default: false + description: |- + Force indicates whether Sveltos should delete and recreate a resource defined in this + PolicyRef/KustomizationRef when an update is rejected with an error that only a + delete+recreate can resolve (eg an invalid combination of fields, or a field enforced + as immutable). By default, such errors are surfaced instead of recreating the resource. + type: boolean helmChartVersion: description: |- HelmChartVersion indicates the chart version of the Helm release diff --git a/lib/crd/configurationbundles.go b/lib/crd/configurationbundles.go index 862094d..9d5adc6 100644 --- a/lib/crd/configurationbundles.go +++ b/lib/crd/configurationbundles.go @@ -57,6 +57,14 @@ spec: type: object spec: properties: + force: + default: false + description: |- + Force indicates whether Sveltos should delete and recreate a resource defined in this + PolicyRef/KustomizationRef when an update is rejected with an error that only a + delete+recreate can resolve (eg an invalid combination of fields, or a field enforced + as immutable). By default, such errors are surfaced instead of recreating the resource. + type: boolean helmChartVersion: description: |- HelmChartVersion indicates the chart version of the Helm release diff --git a/lib/deployer/applier.go b/lib/deployer/applier.go index 77cd0ea..000f139 100644 --- a/lib/deployer/applier.go +++ b/lib/deployer/applier.go @@ -28,6 +28,7 @@ import ( "regexp" "sort" "strings" + "time" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -39,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" utilyaml "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/client-go/dynamic" "k8s.io/client-go/util/retry" @@ -53,8 +55,57 @@ import ( const ( ReasonLabel = "projectsveltos.io/reason" + + recreateDeletePollInterval = 2 * time.Second + recreateDeletePollTimeout = time.Minute ) +// requiresRecreate returns true if err indicates the API server rejected the request +// because of an invalid combination of fields (eg a Deployment with strategy.rollingUpdate +// set while strategy.type is Recreate) or a field enforced as immutable. Either case can +// only be resolved by deleting and recreating the object; a normal server-side apply patch +// cannot clear a field it does not own or change a value the server refuses outright. +func requiresRecreate(err error) bool { + if err == nil { + return false + } + + if apierrors.IsInvalid(err) { + return true + } + + for i := range immutableFieldErrorPatterns { + if immutableFieldErrorPatterns[i].MatchString(err.Error()) { + return true + } + } + + return false +} + +// recreateResource deletes object and waits for it to be gone, so a subsequent create +// does not race against a still-terminating object (eg one with finalizers). +func recreateResource(ctx context.Context, dr dynamic.ResourceInterface, object *unstructured.Unstructured) error { + propagationPolicy := metav1.DeletePropagationBackground + err := dr.Delete(ctx, object.GetName(), metav1.DeleteOptions{PropagationPolicy: &propagationPolicy}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete %s %s for recreate: %w", + object.GetObjectKind().GroupVersionKind().Kind, object.GetName(), err) + } + + return wait.PollUntilContextTimeout(ctx, recreateDeletePollInterval, recreateDeletePollTimeout, true, + func(ctx context.Context) (bool, error) { + _, getErr := dr.Get(ctx, object.GetName(), metav1.GetOptions{}) + if apierrors.IsNotFound(getErr) { + return true, nil + } + if getErr != nil { + return false, getErr + } + return false, nil + }) +} + // CreateNamespace creates a namespace if it does not exist already // No action in DryRun mode. func CreateNamespace(ctx context.Context, clusterClient client.Client, @@ -177,6 +228,13 @@ func GetUnstructured(section []byte, logger logr.Logger) ([]*unstructured.Unstru var ( reCommentLine = regexp.MustCompile(`(?m)^\s*#([^#].*?)$`) reEmptyLine = regexp.MustCompile(`(?m)^\s*$`) + + // immutableFieldErrorPatterns matches error messages returned by custom admission webhooks + // and Kubernetes CEL validation rules that enforce field immutability. + immutableFieldErrorPatterns = []*regexp.Regexp{ + regexp.MustCompile(`.*is\simmutable.*`), + regexp.MustCompile(`.*immutable\sfield.*`), + } ) // removeCommentsAndEmptyLines removes any line containing just YAML comments @@ -443,7 +501,11 @@ func removeDriftExclusionsFields(ctx context.Context, dr dynamic.ResourceInterfa // UpdateResource creates or updates a resource in a Cluster. // No action in DryRun mode. -func UpdateResource(ctx context.Context, dr dynamic.ResourceInterface, isDriftDetection, isDryRun bool, +// When forceRecreate is true, and the apply is rejected with an error that only a +// delete+recreate can resolve (see requiresRecreate), the object is deleted and recreated +// instead of returning the error. This never applies to CustomResourceDefinitions, since +// deleting one cascades to every instance of it. +func UpdateResource(ctx context.Context, dr dynamic.ResourceInterface, isDriftDetection, isDryRun, forceRecreate bool, driftExclusions []libsveltosv1beta1.DriftExclusion, object *unstructured.Unstructured, subresources []string, logger logr.Logger) (*unstructured.Unstructured, error) { @@ -503,6 +565,14 @@ func UpdateResource(ctx context.Context, dr dynamic.ResourceInterface, isDriftDe } return nil }) + + if err != nil && !isDryRun && forceRecreate && requiresRecreate(err) { + l.V(logs.LogInfo).Info(fmt.Sprintf("apply rejected (%v), recreating resource", err)) + if recreateErr := recreateResource(ctx, dr, object); recreateErr != nil { + return nil, recreateErr + } + updatedObject, err = dr.Patch(ctx, object.GetName(), types.ApplyPatchType, data, options) + } } if err != nil { return nil, err diff --git a/lib/deployer/applier_test.go b/lib/deployer/applier_test.go index ab93ff8..3560f9c 100644 --- a/lib/deployer/applier_test.go +++ b/lib/deployer/applier_test.go @@ -17,14 +17,22 @@ limitations under the License. package deployer_test import ( + "context" "fmt" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2/textlogger" libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" "github.com/projectsveltos/libsveltos/lib/deployer" + "github.com/projectsveltos/libsveltos/lib/k8s_utils" ) const ( @@ -679,3 +687,126 @@ status: Expect(result[0].GetKind()).To(Equal("Service")) }) }) + +var _ = Describe("requiresRecreate", func() { + It("returns true for Invalid errors", func() { + err := apierrors.NewInvalid( + schema.GroupKind{Group: appsGroup, Kind: "Deployment"}, "foo", nil) + Expect(deployer.RequiresRecreate(err)).To(BeTrue()) + }) + + It("returns true for immutable field error text", func() { + Expect(deployer.RequiresRecreate(fmt.Errorf("the field xyz is immutable"))).To(BeTrue()) + Expect(deployer.RequiresRecreate(fmt.Errorf("update rejected: immutable field detected"))).To(BeTrue()) + }) + + It("returns false for unrelated errors", func() { + Expect(deployer.RequiresRecreate(apierrors.NewNotFound( + schema.GroupResource{Group: appsGroup, Resource: "deployments"}, "foo"))).To(BeFalse()) + Expect(deployer.RequiresRecreate(fmt.Errorf("connection refused"))).To(BeFalse()) + }) + + It("returns false for nil", func() { + Expect(deployer.RequiresRecreate(nil)).To(BeFalse()) + }) +}) + +const ( + deploymentNoStrategyTemplate = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: %s + namespace: %s +spec: + replicas: 1 + selector: + matchLabels: + app: %s + template: + metadata: + labels: + app: %s + spec: + containers: + - name: main + image: nginx:latest` + + deploymentRecreateStrategyTemplate = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: %s + namespace: %s +spec: + replicas: 1 + strategy: + type: Recreate + selector: + matchLabels: + app: %s + template: + metadata: + labels: + app: %s + spec: + containers: + - name: main + image: nginx:latest` +) + +var _ = Describe("UpdateResource force recreate", func() { + It("recreates a Deployment when a strategy change is rejected by the API server and forceRecreate is set", func() { + name := randomString() + nsName := randomString() + + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: nsName}} + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + logger := textlogger.NewLogger(textlogger.NewConfig()) + + initialYAML := fmt.Sprintf(deploymentNoStrategyTemplate, name, nsName, name, name) + initialObj, err := k8s_utils.GetUnstructured([]byte(initialYAML)) + Expect(err).To(BeNil()) + + dr, err := k8s_utils.GetDynamicResourceInterface(testEnv.Config, initialObj.GroupVersionKind(), nsName) + Expect(err).To(BeNil()) + + // Deployed the same way Sveltos would have before strategy was added: no strategy set, + // so the API server defaults it to RollingUpdate with an explicit rollingUpdate value. + _, err = deployer.UpdateResource(context.TODO(), dr, false, false, false, + nil, initialObj, nil, logger) + Expect(err).To(BeNil()) + + Eventually(func() bool { + current, getErr := dr.Get(context.TODO(), name, metav1.GetOptions{}) + if getErr != nil { + return false + } + _, found, _ := unstructured.NestedMap(current.Object, "spec", "strategy", "rollingUpdate") + return found + }, time.Minute, 5*time.Second).Should(BeTrue()) + + recreateYAML := fmt.Sprintf(deploymentRecreateStrategyTemplate, name, nsName, name, name) + recreateObj, err := k8s_utils.GetUnstructured([]byte(recreateYAML)) + Expect(err).To(BeNil()) + + // Without forceRecreate, the leftover rollingUpdate value conflicts with the new + // strategy.type and the API server rejects the apply. + _, err = deployer.UpdateResource(context.TODO(), dr, false, false, false, + nil, recreateObj, nil, logger) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("rollingUpdate")) + + // With forceRecreate, the object is deleted and recreated to match the new manifest. + updated, err := deployer.UpdateResource(context.TODO(), dr, false, false, true, + nil, recreateObj, nil, logger) + Expect(err).To(BeNil()) + Expect(updated).ToNot(BeNil()) + + strategyType, _, _ := unstructured.NestedString(updated.Object, "spec", "strategy", "type") + Expect(strategyType).To(Equal("Recreate")) + + _, found, _ := unstructured.NestedMap(updated.Object, "spec", "strategy", "rollingUpdate") + Expect(found).To(BeFalse()) + }) +}) diff --git a/lib/deployer/export_test.go b/lib/deployer/export_test.go index 450eca7..3dbd397 100644 --- a/lib/deployer/export_test.go +++ b/lib/deployer/export_test.go @@ -30,6 +30,8 @@ var ( CanDelete = canDelete DeployResourceSummaryInstance = deployResourceSummaryInstance + + RequiresRecreate = requiresRecreate ) func (d *deployer) SetInProgress(inProgress []string) { diff --git a/lib/pullmode/export_test.go b/lib/pullmode/export_test.go index bbb8e00..d898daa 100644 --- a/lib/pullmode/export_test.go +++ b/lib/pullmode/export_test.go @@ -28,6 +28,8 @@ var ( UpdateConfigurationGroup = updateConfigurationGroup ReconcileConfigurationGroup = reconcileConfigurationGroup GetConfigurationGroups = getConfigurationGroups + + ApplyBundleSetters = applyBundleSetters ) const ( diff --git a/lib/pullmode/setters.go b/lib/pullmode/setters.go index 31950fe..564a1ce 100644 --- a/lib/pullmode/setters.go +++ b/lib/pullmode/setters.go @@ -229,6 +229,7 @@ type BundleOptions struct { ReferencedObjectName string ReferencedTier int32 SkipNamespaceCreation bool + Force bool } type BundleOption func(*BundleOptions) @@ -254,7 +255,7 @@ func WithReleaseInfo(namespace, name, repoURL, chartVersion, icon string, } func WithResourceInfo(kind, namespace, name string, - tier int32, skipNamespaceCreation bool) BundleOption { + tier int32, skipNamespaceCreation, force bool) BundleOption { return func(args *BundleOptions) { args.ReferencedObjectKind = kind @@ -262,6 +263,7 @@ func WithResourceInfo(kind, namespace, name string, args.ReferencedObjectName = name args.ReferencedTier = tier args.SkipNamespaceCreation = skipNamespaceCreation + args.Force = force } } @@ -292,6 +294,7 @@ func applyBundleSetters(confBundle *libsveltosv1beta1.ConfigurationBundle, sette confBundle.Spec.ReferencedObjectName = c.ReferencedObjectName confBundle.Spec.ReferenceTier = c.ReferencedTier confBundle.Spec.SkipNamespaceCreation = c.SkipNamespaceCreation + confBundle.Spec.Force = c.Force return confBundle } diff --git a/lib/pullmode/utils_test.go b/lib/pullmode/utils_test.go index 7063564..f53dbe7 100644 --- a/lib/pullmode/utils_test.go +++ b/lib/pullmode/utils_test.go @@ -378,6 +378,35 @@ var _ = Describe("Utils for pullmode APIs", func() { }) }) +var _ = Describe("applyBundleSetters", func() { + It("WithResourceInfo sets Force on the ConfigurationBundle", func() { + bundle := &libsveltosv1beta1.ConfigurationBundle{} + + kind := randomString() + namespace := randomString() + name := randomString() + var tier int32 = 100 + + pullmode.ApplyBundleSetters(bundle, pullmode.WithResourceInfo(kind, namespace, name, tier, true, true)) + + Expect(bundle.Spec.ReferencedObjectKind).To(Equal(kind)) + Expect(bundle.Spec.ReferencedObjectNamespace).To(Equal(namespace)) + Expect(bundle.Spec.ReferencedObjectName).To(Equal(name)) + Expect(bundle.Spec.ReferenceTier).To(Equal(tier)) + Expect(bundle.Spec.SkipNamespaceCreation).To(BeTrue()) + Expect(bundle.Spec.Force).To(BeTrue()) + }) + + It("WithResourceInfo defaults Force to false", func() { + bundle := &libsveltosv1beta1.ConfigurationBundle{} + + pullmode.ApplyBundleSetters(bundle, + pullmode.WithResourceInfo(randomString(), randomString(), randomString(), 100, false, false)) + + Expect(bundle.Spec.Force).To(BeFalse()) + }) +}) + func getResources() []unstructured.Unstructured { namespace := ` apiVersion: v1 kind: Namespace diff --git a/manifests/apiextensions.k8s.io_v1_customresourcedefinition_configurationbundles.lib.projectsveltos.io.yaml b/manifests/apiextensions.k8s.io_v1_customresourcedefinition_configurationbundles.lib.projectsveltos.io.yaml index d26cb64..2e4456f 100644 --- a/manifests/apiextensions.k8s.io_v1_customresourcedefinition_configurationbundles.lib.projectsveltos.io.yaml +++ b/manifests/apiextensions.k8s.io_v1_customresourcedefinition_configurationbundles.lib.projectsveltos.io.yaml @@ -38,6 +38,14 @@ spec: type: object spec: properties: + force: + default: false + description: |- + Force indicates whether Sveltos should delete and recreate a resource defined in this + PolicyRef/KustomizationRef when an update is rejected with an error that only a + delete+recreate can resolve (eg an invalid combination of fields, or a field enforced + as immutable). By default, such errors are surfaced instead of recreating the resource. + type: boolean helmChartVersion: description: |- HelmChartVersion indicates the chart version of the Helm release diff --git a/renovate.json b/renovate.json new file mode 100644 index 0000000..a85e67e --- /dev/null +++ b/renovate.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://docs.renovatebot.com/renovate-schema.json", + "extends": ["config:recommended", ":dependencyDashboard"], + "prHourlyLimit": 0, + "prConcurrentLimit": 0, + "branchConcurrentLimit": 0, + "ignorePaths": ["**/test/**"], + "sveltos": { + "fileMatch": ["^.*\\.yaml$"] + } +}