Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/publish-kubescape-storage-hotfix.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ concurrency:
env:
KUBESCAPE_STORAGE_SOURCE_COMMIT: b35788b68337134fc2514574cde1ba7f1225fd43
IMAGE: ghcr.io/devantler-tech/platform-kubescape-storage
IMAGE_TAG: v0.0.297-sqlite-contention.2-${{ github.sha }}
IMAGE_TAG: v0.0.297-sqlite-contention.3-${{ github.sha }}
PATCH_PATH: k8s/bases/infrastructure/controllers/kubescape/storage-v0.0.297-sqlite-contention.patch

jobs:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,41 @@
diff --git a/pkg/registry/file/containerprofile_processor.go b/pkg/registry/file/containerprofile_processor.go
index db4a8ee1..f31119a7 100644
--- a/pkg/registry/file/containerprofile_processor.go
+++ b/pkg/registry/file/containerprofile_processor.go
@@ -75,7 +75,7 @@ func NewContainerProfileProcessor(cfg config.Config, cleanupHandler *ResourcesCl
Interval: 30 * time.Second,
MaxContainerProfileSize: cfg.MaxApplicationProfileSize,
CollapseSettings: dynamicpathdetector.DefaultCollapseSettings,
- Workers: max(1, DefaultPoolSize/4),
+ Workers: 1,
}
}

diff --git a/pkg/registry/file/containerprofile_processor_test.go b/pkg/registry/file/containerprofile_processor_test.go
index 384d059e..b9b57458 100644
--- a/pkg/registry/file/containerprofile_processor_test.go
+++ b/pkg/registry/file/containerprofile_processor_test.go
@@ -12,6 +12,7 @@ import (
"github.com/armosec/armoapi-go/armotypes"
helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers"
"github.com/kubescape/storage/pkg/apis/softwarecomposition"
+ "github.com/kubescape/storage/pkg/config"
"github.com/kubescape/storage/pkg/generated/clientset/versioned/scheme"
"github.com/kubescape/storage/pkg/utils"
"github.com/spf13/afero"
@@ -23,6 +24,12 @@ import (
"zombiezen.com/go/sqlite/sqlitemigration"
)

+func TestNewContainerProfileProcessorUsesOneMaintenanceWriter(t *testing.T) {
+ processor := NewContainerProfileProcessor(config.Config{}, nil)
+ require.Equal(t, 1, processor.Workers,
+ "multiple background writers can continuously hand off SQLite's writer lock and starve REST persistence")
+}
+
func TestConsolidateData(t *testing.T) {
// Prepare pool and connection
pool := NewTestPool("/tmp")
diff --git a/pkg/registry/file/sqlite.go b/pkg/registry/file/sqlite.go
index 3d706533..c219537a 100644
--- a/pkg/registry/file/sqlite.go
Expand Down Expand Up @@ -61,25 +99,11 @@ index 9a1ba4ed..fb1e6a80 100644
func TestMemoryConn(t *testing.T) {
pod := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
diff --git a/pkg/registry/file/containerprofile_storage.go b/pkg/registry/file/containerprofile_storage.go
index 958c5fd6..1126376e 100644
--- a/pkg/registry/file/containerprofile_storage.go
+++ b/pkg/registry/file/containerprofile_storage.go
@@ -78,7 +78,7 @@ func (c *ContainerProfileStorageImpl) WithConnection(ctx context.Context) (conte
// to commit or rollback based on the error state.
func (c *ContainerProfileStorageImpl) BeginTransaction(ctx context.Context) (func(*error), error) {
conn := ctx.Value(connKey).(*sqlite.Conn)
- return sqlitex.Transaction(conn), nil
+ return sqlitex.ImmediateTransaction(conn)
}

func (c *ContainerProfileStorageImpl) DeleteContainerProfile(ctx context.Context, key string) error {
diff --git a/pkg/registry/file/containerprofile_storage_test.go b/pkg/registry/file/containerprofile_storage_test.go
new file mode 100644
index 00000000..8f3aa946
--- /dev/null
+++ b/pkg/registry/file/containerprofile_storage_test.go
@@ -0,0 +1,47 @@
@@ -0,0 +1,41 @@
+package file
+
+import (
Expand All @@ -91,7 +115,7 @@ index 00000000..8f3aa946
+ "zombiezen.com/go/sqlite/sqlitex"
+)
+
+func TestBeginTransactionSerializesWriters(t *testing.T) {
+func TestBeginTransactionLeavesWriterAvailableDuringReadPhase(t *testing.T) {
+ pool := NewPool(t.TempDir()+"/transactions.sq3", 2)
+ require.NotNil(t, pool)
+ defer func() {
Expand All @@ -108,22 +132,16 @@ index 00000000..8f3aa946
+
+ finishFirst, err := storage.BeginTransaction(firstCtx)
+ require.NoError(t, err)
+ firstConn := firstCtx.Value(connKey).(*sqlite.Conn)
+ require.NoError(t, sqlitex.Execute(firstConn, "SELECT name FROM sqlite_schema LIMIT 1;", nil))
+
+ secondConn := secondCtx.Value(connKey).(*sqlite.Conn)
+ secondConn.SetBusyTimeout(0)
+ if err := sqlitex.Execute(secondConn, "BEGIN IMMEDIATE;", nil); err == nil {
+ require.NoError(t, sqlitex.Execute(secondConn, "ROLLBACK;", nil))
+ var firstErr error
+ finishFirst(&firstErr)
+ t.Fatal("second writer acquired SQLite's write reservation before the first transaction completed")
+ } else {
+ require.Equal(t, sqlite.ResultBusy, sqlite.ErrCode(err).ToPrimary())
+ }
+ require.NoError(t, sqlitex.Execute(secondConn, "BEGIN IMMEDIATE;", nil),
+ "a read-heavy background transaction must not reserve SQLite's sole writer")
+ require.NoError(t, sqlitex.Execute(secondConn, "ROLLBACK;", nil))
+
+ var firstErr error
+ finishFirst(&firstErr)
+ require.NoError(t, firstErr)
+
+ require.NoError(t, sqlitex.Execute(secondConn, "BEGIN IMMEDIATE;", nil))
+ require.NoError(t, sqlitex.Execute(secondConn, "ROLLBACK;", nil))
+}
27 changes: 17 additions & 10 deletions scripts/tests/test-kubescape-storage-hotfix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ command -v yq >/dev/null 2>&1 || fail 'yq v4 is required'
fail 'the workflow image destination drifted'
# The literal GitHub expression is the contract.
# shellcheck disable=SC2016
[[ "$(yq -er '.env.IMAGE_TAG' "${workflow}")" == 'v0.0.297-sqlite-contention.2-${{ github.sha }}' ]] ||
fail 'the workflow image tag must identify the writer-serialization revision'
[[ "$(yq -er '.env.IMAGE_TAG' "${workflow}")" == 'v0.0.297-sqlite-contention.3-${{ github.sha }}' ]] ||
fail 'the workflow image tag must identify the foreground-safe transaction revision'

grep -qF 'repository: kubescape/storage' "${workflow}" ||
fail 'the workflow must check out the upstream storage source explicitly'
Expand Down Expand Up @@ -91,7 +91,7 @@ readonly image_build_index
grep -qF 'cosign sign --yes "${IMAGE}@${DIGEST}"' "${workflow}" ||
fail 'the published compatibility image must be keylessly signed by digest'

[[ "$(grep -c '^diff --git ' "${patch_file}")" == '4' ]] ||
[[ "$(grep -c '^diff --git ' "${patch_file}")" == '5' ]] ||
fail 'the compatibility patch must touch only implementation and regression-test files'
grep -qF 'diff --git a/pkg/registry/file/sqlite.go b/pkg/registry/file/sqlite.go' "${patch_file}" ||
fail 'the compatibility patch does not modify SQLite pool setup'
Expand All @@ -101,14 +101,21 @@ grep -qF 'conn.SetBusyTimeout(60 * time.Second)' "${patch_file}" ||
fail 'the compatibility patch must apply the upstream-reviewed 60-second timeout'
grep -qF 'assert.Equal(t, int64(60000), busyTimeoutMilliseconds)' "${patch_file}" ||
fail 'the compatibility patch must prove the effective SQLite timeout'
grep -qF 'diff --git a/pkg/registry/file/containerprofile_storage.go b/pkg/registry/file/containerprofile_storage.go' "${patch_file}" ||
fail 'the compatibility patch does not serialize container-profile writer transactions'
grep -qF 'diff --git a/pkg/registry/file/containerprofile_processor.go b/pkg/registry/file/containerprofile_processor.go' "${patch_file}" ||
fail 'the compatibility patch does not bound background SQLite writers'
grep -qF 'diff --git a/pkg/registry/file/containerprofile_processor_test.go b/pkg/registry/file/containerprofile_processor_test.go' "${patch_file}" ||
fail 'the compatibility patch lacks a background-writer regression test'
grep -qF 'diff --git a/pkg/registry/file/containerprofile_storage_test.go b/pkg/registry/file/containerprofile_storage_test.go' "${patch_file}" ||
fail 'the compatibility patch lacks a writer-serialization regression test'
grep -qF 'return sqlitex.ImmediateTransaction(conn)' "${patch_file}" ||
fail 'container-profile transactions must reserve SQLite write access before reading'
grep -qF 'TestBeginTransactionSerializesWriters' "${patch_file}" ||
fail 'the compatibility patch must prove concurrent writers serialize'
fail 'the compatibility patch lacks a foreground-writer regression test'
grep -qF 'Workers: 1' "${patch_file}" ||
fail 'container-profile maintenance must use one background writer'
grep -qF 'TestNewContainerProfileProcessorUsesOneMaintenanceWriter' "${patch_file}" ||
fail 'the compatibility patch must prove background writer concurrency is bounded'
grep -qF 'TestBeginTransactionLeavesWriterAvailableDuringReadPhase' "${patch_file}" ||
fail 'the compatibility patch must prove profile reads do not reserve SQLite write access'
if grep -qF 'ImmediateTransaction' "${patch_file}"; then
fail 'read-heavy profile transactions must not reserve SQLite write access before their write phase'
fi

storage_repository="$(yq -er '
.spec.values.storage.image.repository |
Expand Down
Loading