Ensure rpms/gpg-keys is not empty if present (#385)

* Exit early if gpg-keys directory is empty Signed-off-by: Atanas Dinov <[email protected]> * Revise tests Signed-off-by: Atanas Dinov <[email protected]> * Update release notes Signed-off-by: Atanas Dinov <[email protected]> --------- Signed-off-by: Atanas Dinov <[email protected]>
suse-edge · Apr 15, 2024 · d66e74b · d66e74b
1 parent 6d128fb
commit d66e74b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 12 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -20,6 +20,7 @@
 * [#362](https://github.com/suse-edge/edge-image-builder/issues/362) - Helm templating failure
 * [#365](https://github.com/suse-edge/edge-image-builder/issues/365) - Unable to locate downloaded Helm charts
 * [#374](https://github.com/suse-edge/edge-image-builder/issues/374) - Enable SELinux support for Kubernetes agents if servers enforce it
+* [#381](https://github.com/suse-edge/edge-image-builder/issues/381) - Empty gpg-keys directory passes GPG enablement only to fail during the dependency resolution
 
 ---
 

diff --git a/pkg/combustion/rpm.go b/pkg/combustion/rpm.go
@@ -183,18 +183,22 @@ func fetchLocalRPMConfig(ctx *image.Context) (*image.LocalRPMConfig, error) {
 	gpgCheckDisabled := ctx.ImageDefinition.OperatingSystem.Packages.NoGPGCheck
 	gpgPath := GPGKeysPath(ctx)
 
-	if _, err := os.Stat(gpgPath); err == nil {
+	if entries, err := os.ReadDir(gpgPath); err == nil {
 		if gpgCheckDisabled {
 			return nil, fmt.Errorf("found existing '%s' directory, but GPG validation is disabled", gpgDir)
 		}
 
+		if len(entries) == 0 {
+			return nil, fmt.Errorf("'%s' directory exists but it is empty", gpgDir)
+		}
+
 		localRPMConfig.GPGKeysPath = gpgPath
 	} else if !gpgCheckDisabled {
 		if errors.Is(err, fs.ErrNotExist) {
 			return nil, fmt.Errorf("GPG validation is enabled, but '%s' directory is missing for side-loaded RPMs", gpgDir)
 		}
 
-		return nil, fmt.Errorf("describing GPG directory at '%s': %w", gpgPath, err)
+		return nil, fmt.Errorf("reading GPG directory at '%s': %w", gpgPath, err)
 	}
 
 	return localRPMConfig, nil

diff --git a/pkg/combustion/rpm_test.go b/pkg/combustion/rpm_test.go
@@ -37,7 +37,7 @@ func (mr mockRPMRepoCreator) Create(path string) error {
 	panic("not implemented")
 }
 
-func TestSkipRPMComponentTrue(t *testing.T) {
+func TestSkipRPMComponent_InvalidDefinition(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -80,7 +80,7 @@ func TestSkipRPMComponentTrue(t *testing.T) {
 
 }
 
-func TestSkipRPMComponentProvidedPKGList(t *testing.T) {
+func TestSkipRPMComponent_PopulatedPackageList(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -91,7 +91,7 @@ func TestSkipRPMComponentProvidedPKGList(t *testing.T) {
 	assert.False(t, SkipRPMComponent(ctx))
 }
 
-func TestSkipRPMComponentRPMDirNoRPMs(t *testing.T) {
+func TestSkipRPMComponent_EmptyRPMDir(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -104,7 +104,7 @@ func TestSkipRPMComponentRPMDirNoRPMs(t *testing.T) {
 	assert.True(t, SkipRPMComponent(ctx))
 }
 
-func TestSkipRPMComponentFullConfig(t *testing.T) {
+func TestSkipRPMComponent_FullConfig(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -127,7 +127,7 @@ func TestSkipRPMComponentFullConfig(t *testing.T) {
 	assert.False(t, SkipRPMComponent(ctx))
 }
 
-func TestConfigureRPMSSkipComponent(t *testing.T) {
+func TestConfigureRPMs_Skipped(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -137,7 +137,7 @@ func TestConfigureRPMSSkipComponent(t *testing.T) {
 	assert.Nil(t, scripts)
 }
 
-func TestConfigureRPMSError(t *testing.T) {
+func TestConfigureRPMs_ResolutionFailures(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
@@ -222,18 +222,18 @@ func TestConfigureRPMSError(t *testing.T) {
 	}
 }
 
-func TestConfigureRPMSGPGDirError(t *testing.T) {
+func TestConfigureRPMs_GPGFailures(t *testing.T) {
 	ctx, teardown := setupContext(t)
 	defer teardown()
 
 	rpmDir := filepath.Join(ctx.ImageConfigDir, rpmDir)
 	require.NoError(t, os.Mkdir(rpmDir, 0o755))
-	_, err := os.Create(filepath.Join(rpmDir, "test.rpm"))
-	require.NoError(t, err)
 	defer func() {
 		require.NoError(t, os.RemoveAll(rpmDir))
 	}()
 
+	require.NoError(t, os.WriteFile(filepath.Join(rpmDir, "test.rpm"), nil, 0o600))
+
 	tests := []struct {
 		name         string
 		expectedErr  string
@@ -248,6 +248,11 @@ func TestConfigureRPMSGPGDirError(t *testing.T) {
 			},
 			expectedErr: "fetching local RPM config: found existing 'gpg-keys' directory, but GPG validation is disabled",
 		},
+		{
+			name:         "Enabled GPG validation, but empty GPG dir",
+			createGPGDir: true,
+			expectedErr:  "fetching local RPM config: 'gpg-keys' directory exists but it is empty",
+		},
 		{
 			name:        "Enabled GPG validation, but missing GPG dir",
 			pkgs:        image.Packages{},
@@ -273,7 +278,7 @@ func TestConfigureRPMSGPGDirError(t *testing.T) {
 	}
 }
 
-func TestConfigureRPMSSuccessfulConfig(t *testing.T) {
+func TestConfigureRPMs_SuccessfulConfig(t *testing.T) {
 	expectedRepoName := "bar"
 	expectedDir := "/foo/bar"
 	expectedPkg := []string{"foo", "bar"}
@@ -299,6 +304,8 @@ func TestConfigureRPMSSuccessfulConfig(t *testing.T) {
 	gpgDir := filepath.Join(rpmDir, gpgDir)
 	require.NoError(t, os.Mkdir(gpgDir, 0o755))
 
+	require.NoError(t, os.WriteFile(filepath.Join(gpgDir, "some-key"), nil, 0o600))
+
 	ctx.RPMRepoCreator = mockRPMRepoCreator{
 		createFunc: func(path string) error {
 			return nil