From 3065a65009cc1ca4a2f38c2440c777e1888b5e4f Mon Sep 17 00:00:00 2001 From: Andrea Waltlova Date: Mon, 31 Jul 2023 14:54:58 +0200 Subject: [PATCH 1/2] Rewrite all tests with table driven approach test cases are now easily readable Signed-off-by: Andrea Waltlova --- src/logger_test.go | 107 ++++++++++------ src/runner_test.go | 159 +++++++++++++---------- src/server_test.go | 237 +++++++++++++++++++++++------------ src/util_test.go | 305 ++++++++++++++++++++++++++------------------- 4 files changed, 493 insertions(+), 315 deletions(-) diff --git a/src/logger_test.go b/src/logger_test.go index 3666111..ea2de56 100644 --- a/src/logger_test.go +++ b/src/logger_test.go @@ -9,50 +9,75 @@ import ( ) func TestSetupLogger(t *testing.T) { - // Create a temporary directory for the log folder - logFolderName := "log-test" - logFileName := "log-file" - defer os.RemoveAll(logFolderName) - - // Test case 1: YGG_LOG_LEVEL doesn't exist, info level should be set to info - setupLogger(logFolderName, logFileName) - level := log.CurrentLevel() - if log.CurrentLevel() != log.LevelInfo { - t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelInfo, level) - } - // Test case 2: Unparsable level in env variable - os.Setenv("YGG_LOG_LEVEL", "....") - setupLogger(logFolderName, logFileName) - level = log.CurrentLevel() - if log.CurrentLevel() != log.LevelInfo { - t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelInfo, level) + testCases := []struct { + name string + logLevelEnv string + expectedLevel log.Level + expectedFlags int + }{ + { + name: "YGG_LOG_LEVEL doesn't exist, info level should be set to info", + logLevelEnv: "", + expectedLevel: log.LevelInfo, + expectedFlags: log.Lshortfile | log.LstdFlags, + }, + { + name: "Unparsable level in env variable", + logLevelEnv: "....", + expectedLevel: log.LevelInfo, + expectedFlags: log.Lshortfile | log.LstdFlags, + }, + { + name: "Everything set up correctly", + logLevelEnv: "debug", + expectedLevel: log.LevelDebug, + expectedFlags: log.Lshortfile | log.LstdFlags, + }, } - // Test case 3: Everything set up correctly - os.Setenv("YGG_LOG_LEVEL", "debug") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a temporary directory for the log folder + logFolderName := "log-test" + logFileName := "log-file" + defer os.RemoveAll(logFolderName) - logfile := setupLogger(logFolderName, logFileName) - if _, err := os.Stat(logFolderName); os.IsNotExist(err) { - t.Errorf("Log folder not created: %v", err) - } - logFilePath := filepath.Join(logFolderName, logFileName) - if _, err := os.Stat(logFilePath); os.IsNotExist(err) { - t.Errorf("Log file not created: %v", err) - } - level = log.CurrentLevel() - if level != log.LevelDebug { - t.Errorf("Incorrect log level. Expected: %v, Got: %v", log.LevelDebug, level) - } - flags := log.Flags() - expectedFlags := log.Lshortfile | log.LstdFlags - if flags != expectedFlags { - t.Errorf("Incorrect log flags. Expected: %v, Got: %v", expectedFlags, flags) - } + // Set YGG_LOG_LEVEL environment variable + if tc.logLevelEnv != "" { + os.Setenv("YGG_LOG_LEVEL", tc.logLevelEnv) + } + + logfile := setupLogger(logFolderName, logFileName) + + // Verify log level + level := log.CurrentLevel() + if level != tc.expectedLevel { + t.Errorf("Incorrect log level. Expected: %v, Got: %v", tc.expectedLevel, level) + } + + // Verify log file and folder + if _, err := os.Stat(logFolderName); os.IsNotExist(err) { + t.Errorf("Log folder not created: %v", err) + } + logFilePath := filepath.Join(logFolderName, logFileName) + if _, err := os.Stat(logFilePath); os.IsNotExist(err) { + t.Errorf("Log file not created: %v", err) + } + + // Verify log flags + flags := log.Flags() + if flags != tc.expectedFlags { + t.Errorf("Incorrect log flags. Expected: %v, Got: %v", tc.expectedFlags, flags) + } - // Cleanup - close the log file - defer os.Unsetenv("YGG_LOG_LEVEL") - err := logfile.Close() - if err != nil { - t.Errorf("Failed to close log file: %v", err) + // Cleanup - close the log file and unset the YGG_LOG_LEVEL environment variable + defer func() { + os.Unsetenv("YGG_LOG_LEVEL") + err := logfile.Close() + if err != nil { + t.Errorf("Failed to close log file: %v", err) + } + }() + }) } } diff --git a/src/runner_test.go b/src/runner_test.go index 4296666..8c85fef 100644 --- a/src/runner_test.go +++ b/src/runner_test.go @@ -7,85 +7,116 @@ import ( ) func TestProcessSignedScript(t *testing.T) { - shouldVerifyYaml := false - shouldDoInsightsCoreGPGCheck := false - temporaryWorkerDirectory := "test-dir" - config = &Config{ - VerifyYAML: &shouldVerifyYaml, - TemporaryWorkerDirectory: &temporaryWorkerDirectory, - InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, - } - - defer os.RemoveAll(temporaryWorkerDirectory) - - // Test case 1: verification disabled, no yaml data supplied = empty output - yamlData := []byte{} - expectedResult := "" - result := processSignedScript(yamlData) - if result != expectedResult { - t.Errorf("Expected %q, but got %q", expectedResult, result) - } - - // Test case 2: verification disabled, yaml data supplied = non-empty output - yamlData = []byte(` + testCases := []struct { + name string + verifyYAML bool + yamlData []byte + expectedResult string + }{ + { + name: "verification disabled, no yaml data supplied = empty output", + verifyYAML: false, + yamlData: []byte{}, + expectedResult: "", + }, + { + name: "verification disabled, yaml data supplied = non-empty output", + verifyYAML: false, + yamlData: []byte(` +vars: + insights_signature: "invalid-signature" + insights_signature_exclude: "/vars/insights_signature,/vars/content_vars" + content: | + #!/bin/sh + echo "$RHC_WORKER_FOO $RHC_WORKER_BAR!" + content_vars: + FOO: Hello + BAR: World`), + expectedResult: "Hello World!\n", + }, + { + name: "verification enabled, invalid signature = error msg returned", + verifyYAML: true, + yamlData: []byte(` vars: - _insights_signature: "invalid-signature" - _insights_signature_exclude: "/vars/insights_signature,/vars/content_vars" + insights_signature: "invalid-signature" + insights_signature_exclude: "/vars/insights_signature,/vars/content_vars" content: | #!/bin/sh echo "$RHC_WORKER_FOO $RHC_WORKER_BAR!" content_vars: FOO: Hello - BAR: World`) - expectedResult = "Hello World!\n" - result = processSignedScript(yamlData) - if result != expectedResult { - t.Errorf("Expected %q, but got %q", expectedResult, result) + BAR: World`), + expectedResult: "Signature of yaml file is invalid", + }, } - // FIXME: This is false success because verification fails on missing insighs-client - // Test case 3: verification enabled, invalid signature = error msg returned - shouldVerifyYaml = true - shouldDoInsightsCoreGPGCheck = true - expectedResult = "Signature of yaml file is invalid" - result = processSignedScript(yamlData) - if result != expectedResult { - t.Errorf("Expected %q, but got %q", expectedResult, result) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + shouldVerifyYaml := tc.verifyYAML + shouldDoInsightsCoreGPGCheck := tc.verifyYAML // Assume the same value for simplicity + temporaryWorkerDirectory := t.TempDir() + config = &Config{ + VerifyYAML: &shouldVerifyYaml, + TemporaryWorkerDirectory: &temporaryWorkerDirectory, + InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, + } + + defer os.RemoveAll(temporaryWorkerDirectory) + + result := processSignedScript(tc.yamlData) + if result != tc.expectedResult { + t.Errorf("Expected %q, but got %q", tc.expectedResult, result) + } + }) } } func TestVerifyYamlFile(t *testing.T) { - shouldVerifyYaml := false - shouldDoInsightsCoreGPGCheck := false - - config = &Config{ - VerifyYAML: &shouldVerifyYaml, - InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, - } - // Test case 1: verification disabled - expectedResult := true - result := verifyYamlFile([]byte{}) - if result != expectedResult { - t.Errorf("Expected %v, but got %v", expectedResult, result) + testCases := []struct { + name string + verifyYAML bool + yamlData []byte + expectedResult bool + }{ + { + name: "verification disabled", + verifyYAML: false, + yamlData: []byte{}, + expectedResult: true, + }, + // FIXME: This should succedd but now verification fails on missing insighs-client + // We also need valid signature + { + name: "verification enabled and verification succeeds", + verifyYAML: true, + yamlData: []byte("valid-yaml"), + expectedResult: false, + }, + // FIXME: Valid test case but fails because of missing insights-client + { + name: "verification is enabled and verification fails", + verifyYAML: true, + yamlData: []byte("invalid-yaml"), + expectedResult: false, + }, } - // Test case 2: verification enabled and verification succeeds - shouldVerifyYaml = true - // FIXME: This should succedd but now verification fails on missing insighs-client - // We also need valid signature - expectedResult = false - result = verifyYamlFile([]byte("valid-yaml")) - if result != expectedResult { - t.Errorf("Expected %v, but got %v", expectedResult, result) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + shouldVerifyYaml := tc.verifyYAML + shouldDoInsightsCoreGPGCheck := false // Reset to false for each test case + + config = &Config{ + VerifyYAML: &shouldVerifyYaml, + InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, + } - // FIXME: Valid test case but fails because of missing insights-client - // Test case 3: sverification is enabled and verification fails - // shouldVerifyYaml = true - expectedResult = false - result = verifyYamlFile([]byte("invalid-yaml")) // Replace with your YAML data - if result != expectedResult { - t.Errorf("Expected %v, but got %v", expectedResult, result) + result := verifyYamlFile(tc.yamlData) + if result != tc.expectedResult { + t.Errorf("Expected %v, but got %v", tc.expectedResult, result) + } + }) } } diff --git a/src/server_test.go b/src/server_test.go index a21a8a1..01d1def 100644 --- a/src/server_test.go +++ b/src/server_test.go @@ -9,66 +9,89 @@ import ( ) func TestCreateDataMessage(t *testing.T) { - // Test case 1: commandOutput is not empty - commandOutput := "Output of the command" - metadata := map[string]string{ - "correlation_id": "123", - "return_content_type": "application/json", - "return_url": "example.com", - } - directive := "Directive value" - messageID := "Message ID" - - data := createDataMessage(commandOutput, metadata, directive, messageID) - - expectedCorrelationID := "123" - if data.Metadata["correlation_id"] != expectedCorrelationID { - t.Errorf("Expected correlation_id to be %s, but got %s", expectedCorrelationID, data.Metadata["correlation_id"]) - } - - expectedMessageType := "multipart/form-data" - if !strings.HasPrefix(data.Metadata["Content-Type"], expectedMessageType) { - t.Errorf("Expected Content-Type to have prefix %s, but got %s", expectedMessageType, data.Metadata["Content-Type"]) - } - - expectedDirective := "example.com" - if data.Directive != expectedDirective { - t.Errorf("Expected Directive to be %s, but got %s", expectedDirective, data.Directive) - } - - expectedMessageID := messageID - if data.ResponseTo != expectedMessageID { - t.Errorf("Expected ResponseTo to be %s, but got %s", expectedMessageID, data.ResponseTo) + testCases := []struct { + name string + commandOutput string + metadata map[string]string + directive string + messageID string + expectedCorrelationID string + expectedMessageType string + expectedDirective string + expectedMessageID string + }{ + { + name: "commandOutput is not empty", + commandOutput: "Output of the command", + metadata: map[string]string{ + "correlation_id": "123", + "return_content_type": "application/json", + "return_url": "example.com", + }, + directive: "Directive value", + messageID: "Message ID", + expectedCorrelationID: "123", + expectedMessageType: "multipart/form-data", + expectedDirective: "example.com", + expectedMessageID: "Message ID", + }, + { + name: "commandOutput is empty", + commandOutput: "", + metadata: map[string]string{ + "correlation_id": "456", + "return_content_type": "text/plain", + "return_url": "example.org", + }, + directive: "Another directive", + messageID: "Another message ID", + expectedCorrelationID: "456", + expectedMessageType: "multipart/form-data", + expectedDirective: "Another directive", + expectedMessageID: "Another message ID", + }, } - commandOutput = "" - data = createDataMessage(commandOutput, metadata, directive, messageID) - if data.Directive != directive { - t.Errorf("Expected Directive to be %s, but got %s", directive, data.Directive) - } - if string(data.Content) != "" { - t.Errorf("Expected Content to be empty, but got %s", data.Content) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + data := createDataMessage(tc.commandOutput, tc.metadata, tc.directive, tc.messageID) + + if tc.commandOutput == "" { + // Checks for commandOutput being empty + if data.Directive != tc.directive { + t.Errorf("Expected Directive to be %s, but got %s", tc.directive, data.Directive) + } + if string(data.Content) != "" { + t.Errorf("Expected Content to be empty, but got %s", data.Content) + } + } else { + // Checks for commandOutput being non-empty + if data.Metadata["correlation_id"] != tc.expectedCorrelationID { + t.Errorf("Expected correlation_id to be %s, but got %s", tc.expectedCorrelationID, data.Metadata["correlation_id"]) + } + + expectedMessageType := "multipart/form-data" + if !strings.HasPrefix(data.Metadata["Content-Type"], expectedMessageType) { + t.Errorf("Expected Content-Type to have prefix %s, but got %s", expectedMessageType, data.Metadata["Content-Type"]) + } + + if data.Directive != tc.expectedDirective { + t.Errorf("Expected Directive to be %s, but got %s", tc.expectedDirective, data.Directive) + } + + if data.ResponseTo != tc.expectedMessageID { + t.Errorf("Expected ResponseTo to be %s, but got %s", tc.expectedMessageID, data.ResponseTo) + } + } + }) } } func TestProcessData(t *testing.T) { - // FIXME: this should ideally test that all correct functions are called - // Probably easiest would be to move them to interface and then make the function argument take mock interface - yggdDispatchSocketAddr = "mock-target" - - shouldVerifyYaml := false - shouldDoInsightsCoreGPGCheck := false - temporaryWorkerDirectory := "test-dir" - config = &Config{ - VerifyYAML: &shouldVerifyYaml, - TemporaryWorkerDirectory: &temporaryWorkerDirectory, - InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, - } - - yamlData := []byte(` + testYAMLData := []byte(` vars: - _insights_signature: "invalid-signature" - _insights_signature_exclude: "/vars/insights_signature,/vars/content_vars" + insights_signature: "invalid-signature" + insights_signature_exclude: "/vars/insights_signature,/vars/content_vars" content: | #!/bin/sh echo "$RHC_WORKER_FOO $RHC_WORKER_BAR!" @@ -76,42 +99,94 @@ vars: FOO: Hello BAR: World`) - returnURL := "bar" - testData := &pb.Data{ - Content: yamlData, - Metadata: map[string]string{ - "return_content_type": "foo", - "return_url": returnURL, - "correlation_id": "000", + testCases := []struct { + name string + yamlData []byte + expectedOutput string + expectedDirective string + expectedReturnContent string + }{ + { + name: "Expected data are present in result data", + yamlData: testYAMLData, + expectedOutput: "Hello World!", + expectedDirective: "bar", + expectedReturnContent: "foo", }, - Directive: "Your directive", - MessageId: "Your message ID", - } - - data := processData(testData) - expectedOutput := "Hello World!" - - if !strings.Contains(string(data.GetContent()), expectedOutput) { - t.Errorf("Expected content to contain '%s', but it didn't", expectedOutput) } - if data.GetDirective() != returnURL { - t.Errorf("Expected directive to contain '%s', but it didn't", returnURL) + // Set the mock target address + yggdDispatchSocketAddr = "mock-target" + defer func() { + yggdDispatchSocketAddr = "" // Reset the mock target address after the tests + }() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + shouldVerifyYaml := false + shouldDoInsightsCoreGPGCheck := false + temporaryWorkerDirectory := t.TempDir() + config = &Config{ + VerifyYAML: &shouldVerifyYaml, + TemporaryWorkerDirectory: &temporaryWorkerDirectory, + InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck, + } + + returnURL := "bar" + testData := &pb.Data{ + Content: tc.yamlData, + Metadata: map[string]string{ + "return_content_type": "foo", + "return_url": returnURL, + "correlation_id": "000", + }, + Directive: "Your directive", + MessageId: "Your message ID", + } + + data := processData(testData) + + if !strings.Contains(string(data.GetContent()), tc.expectedOutput) { + t.Errorf("Expected content to contain '%s', but it didn't", tc.expectedOutput) + } + + if data.GetDirective() != tc.expectedDirective { + t.Errorf("Expected directive to contain '%s', but it didn't", tc.expectedDirective) + } + + if data.GetMetadata()["return_content_type"] != tc.expectedReturnContent { + t.Errorf("Expected return content type to contain '%s', but it didn't", tc.expectedReturnContent) + } + }) } } func TestSendDataToDispatcher(t *testing.T) { - // Tests only that the function doesn't modify data sent to dispatcher - - yggdDispatchSocketAddr = "mock-target" - - testData := &pb.Data{ - MessageId: uuid.New().String(), - ResponseTo: "mock-id", + testCases := []struct { + name string + testData *pb.Data + }{ + { + name: "Data are not changed before sending them to dispatcher", + testData: &pb.Data{ + MessageId: uuid.New().String(), + ResponseTo: "mock-id", + }, + }, } - data := sendDataToDispatcher(testData) - if data != testData { - t.Errorf("Function should NOT change data before sent, but it did: %s", data) + // Set the mock target address + yggdDispatchSocketAddr = "mock-target" + defer func() { + yggdDispatchSocketAddr = "" // Reset the mock target address after the tests + }() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + data := sendDataToDispatcher(tc.testData) + if data != tc.testData { + t.Errorf("Function should NOT change data before sending, but it did: %s", data) + } + }) } } diff --git a/src/util_test.go b/src/util_test.go index e5c7482..d0f8ef2 100644 --- a/src/util_test.go +++ b/src/util_test.go @@ -9,76 +9,117 @@ import ( ) func TestConstructMetadata(t *testing.T) { - receivedMetadata := map[string]string{ - "Key1": "Value1", - "Key2": "Value2", - } - contentType := "application/json" - - expectedMetadata := map[string]string{ - "Content-Type": "application/json", - "Key1": "Value1", - "Key2": "Value2", - } - - result := constructMetadata(receivedMetadata, contentType) - - if !reflect.DeepEqual(result, expectedMetadata) { - t.Errorf("Unexpected metadata. Expected: %v, but got: %v", expectedMetadata, result) + testCases := []struct { + name string + receivedMetadata map[string]string + contentType string + expectedMetadata map[string]string + }{ + { + name: "Metadata created with expected values", + receivedMetadata: map[string]string{ + "Key1": "Value1", + "Key2": "Value2", + }, + contentType: "application/json", + expectedMetadata: map[string]string{ + "Content-Type": "application/json", + "Key1": "Value1", + "Key2": "Value2", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := constructMetadata(tc.receivedMetadata, tc.contentType) + + if !reflect.DeepEqual(result, tc.expectedMetadata) { + t.Errorf("Unexpected metadata. Expected: %v, but got: %v", tc.expectedMetadata, result) + } + }) } } func TestGetOutputFile(t *testing.T) { - stdout := "Hello, world!" - correlationID := "12345" - contentType := "application/json" - - body, boundary := getOutputFile(stdout, correlationID, contentType) - - // Verify that the boundary is not empty - if boundary == "" { - t.Error("Boundary should not be empty") - } - - expectedPayload := fmt.Sprintf(`{"correlation_id":"%s","stdout":"%s"}`, correlationID, stdout) - gotTrimmed := strings.TrimSpace(body.String()) - - // Verify that the body contains the expected data - if !strings.Contains(gotTrimmed, expectedPayload) { - t.Errorf("Unexpected body payload. Expected to contain: %s, Got: %s", expectedPayload, body.String()) + testCases := []struct { + name string + stdout string + correlationID string + contentType string + }{ + { + name: "Output file contains expected data", + stdout: "Hello, world!", + correlationID: "12345", + contentType: "application/json", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + body, boundary := getOutputFile(tc.stdout, tc.correlationID, tc.contentType) + + // Verify that the boundary is not empty + if boundary == "" { + t.Error("Boundary should not be empty") + } + + expectedPayload := fmt.Sprintf(`{"correlation_id":"%s","stdout":"%s"}`, tc.correlationID, tc.stdout) + gotTrimmed := strings.TrimSpace(body.String()) + + // Verify that the body contains the expected data + if !strings.Contains(gotTrimmed, expectedPayload) { + t.Errorf("Unexpected body payload. Expected to contain: %s, Got: %s", expectedPayload, body.String()) + } + prefix := fmt.Sprintf(`--%s`, boundary) + if !strings.HasPrefix(gotTrimmed, prefix) { + t.Errorf("Unexpected body payload. Expected to have prefix: %s, Got: %s", prefix, body.String()) + } + suffix := fmt.Sprintf(`--%s--`, boundary) + if !strings.HasSuffix(gotTrimmed, suffix) { + t.Errorf("Unexpected body payload. Expected to have suffix: %s, Got: %s", suffix, gotTrimmed) + } + // TODO: test that content type is also there + }) } - prefix := fmt.Sprintf(`--%s`, boundary) - if !strings.HasPrefix(gotTrimmed, prefix) { - t.Errorf("Unexpected body payload. Expected to have prefix: %s, Got: %s", prefix, body.String()) - } - suffix := fmt.Sprintf(`--%s--`, boundary) - if !strings.HasSuffix(gotTrimmed, suffix) { - t.Errorf("Unexpected body payload. Expected to have suffix: %s, Got: %s", suffix, gotTrimmed) - } - // TODO: test that content type is also there } func TestWriteFileToTemporaryDir(t *testing.T) { - // Create a temporary directory for testing - tempDirPath := "test-dir" - defer os.RemoveAll(tempDirPath) - - data := []byte("test data") - filePath := writeFileToTemporaryDir(data, tempDirPath) - - // Assert that the file exists - _, err := os.Stat(filePath) - if err != nil { - t.Errorf("Expected file to be created, got error: %v", err) - } - - // Assert that the file contains the expected data - fileContent, err := os.ReadFile(filePath) - if err != nil { - t.Errorf("Failed to read file content: %v", err) - } - if string(fileContent) != string(data) { - t.Errorf("Expected file content: %s, got: %s", string(data), string(fileContent)) + testCases := []struct { + name string + data []byte + expected string + }{ + { + name: "File is created and contains expected data", + data: []byte("test data"), + expected: "test data", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a temporary directory for testing + tempDirPath := t.TempDir() + + filePath := writeFileToTemporaryDir(tc.data, tempDirPath) + + // Assert that the file exists + _, err := os.Stat(filePath) + if err != nil { + t.Errorf("Expected file to be created, got error: %v", err) + } + + // Assert that the file contains the expected data + fileContent, err := os.ReadFile(filePath) + if err != nil { + t.Errorf("Failed to read file content: %v", err) + } + if string(fileContent) != tc.expected { + t.Errorf("Expected file content: %s, got: %s", tc.expected, string(fileContent)) + } + }) } } @@ -97,73 +138,6 @@ func createTempYAMLFile(content string) (string, error) { return tempFile.Name(), nil } -func TestLoadConfigOrDefault(t *testing.T) { - expectedConfig := &Config{ - Directive: strPtr("rhc-worker-bash"), - VerifyYAML: boolPtr(true), - InsightsCoreGPGCheck: boolPtr(true), - TemporaryWorkerDirectory: strPtr("/var/lib/rhc-worker-bash"), - } - // Test case 1: No config present, defaults set - config := loadConfigOrDefault("foo-bar") - - if !compareConfigs(config, expectedConfig) { - t.Errorf("Loaded config does not match expected config") - } - - // Test case 2: Valid YAML file with all values present - yamlData := ` -directive: "rhc-worker-bash" -verify_yaml: true -verify_yaml_version_check: true -insights_core_gpg_check: true -temporary_worker_directory: "/var/lib/rhc-worker-bash" -` - filePath, err := createTempYAMLFile(yamlData) - if err != nil { - t.Fatalf("Failed to create temporary YAML file: %v", err) - } - defer os.Remove(filePath) - - config = loadConfigOrDefault(filePath) - - if !compareConfigs(config, expectedConfig) { - t.Errorf("Loaded config does not match expected config") - } - - // Test case 3: Valid YAML file with missing values - yamlData = ` -directive: "rhc-worker-bash" -` - filePath, err = createTempYAMLFile(yamlData) - if err != nil { - t.Fatalf("Failed to create temporary YAML file: %v", err) - } - defer os.Remove(filePath) - - config = loadConfigOrDefault(filePath) - - if !compareConfigs(config, expectedConfig) { - t.Errorf("Loaded config does not match expected config") - } - - // Test case 4: Invalid YAML file - default config created - yamlData = ` -invalid_yaml_data -` - filePath, err = createTempYAMLFile(yamlData) - if err != nil { - t.Fatalf("Failed to create temporary YAML file: %v", err) - } - defer os.Remove(filePath) - - config = loadConfigOrDefault(filePath) - - if !compareConfigs(config, expectedConfig) { - t.Errorf("Loaded config does not match expected config") - } -} - // Helper function to compare two Config structs. func compareConfigs(c1, c2 *Config) bool { return *c1.Directive == *c2.Directive && @@ -180,3 +154,76 @@ func strPtr(s string) *string { func boolPtr(b bool) *bool { return &b } + +// Test YAML data +const validYAMLData = ` +directive: "rhc-worker-bash" +verify_yaml: true +verify_yaml_version_check: true +insights_core_gpg_check: true +temporary_worker_directory: "/var/lib/rhc-worker-bash" +` + +const validYAMLDataMissingValues = ` +directive: "rhc-worker-bash" +` + +func TestLoadConfigOrDefault(t *testing.T) { + expectedConfig := &Config{ + Directive: strPtr("rhc-worker-bash"), + VerifyYAML: boolPtr(true), + InsightsCoreGPGCheck: boolPtr(true), + TemporaryWorkerDirectory: strPtr("/var/lib/rhc-worker-bash"), + } + + testCases := []struct { + name string + yamlData string + isValidYAML bool + }{ + { + name: "No config present, defaults set", + yamlData: "", + isValidYAML: false, + }, + { + name: "Valid YAML file with all values present", + yamlData: validYAMLData, + isValidYAML: true, + }, + { + name: "Valid YAML file with missing values", + yamlData: validYAMLDataMissingValues, + isValidYAML: true, + }, + { + name: "Invalid YAML file - default config created", + yamlData: "invalid_yaml_data", + isValidYAML: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + filePath, err := createTempYAMLFile(tc.yamlData) + if err != nil { + t.Fatalf("Failed to create temporary YAML file: %v", err) + } + defer os.Remove(filePath) + + config := loadConfigOrDefault(filePath) + + // Verify if the YAML is valid and matches the expected config + if tc.isValidYAML { + if !compareConfigs(config, expectedConfig) { + t.Errorf("Loaded config does not match expected config") + } + } else { + // If the YAML is invalid, a default config should be created + if !compareConfigs(config, expectedConfig) { + t.Errorf("Loaded config does not match the default config") + } + } + }) + } +} From c9f5fb410f67641205e1ef7eb99e5048a4dae704 Mon Sep 17 00:00:00 2001 From: Andrea Waltlova Date: Tue, 1 Aug 2023 09:53:57 +0200 Subject: [PATCH 2/2] Mock insights-client call during verification tests Signed-off-by: Andrea Waltlova --- src/runner.go | 15 ++++++++------ src/runner_test.go | 49 +++++++++++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/runner.go b/src/runner.go index cead650..e62db9b 100644 --- a/src/runner.go +++ b/src/runner.go @@ -20,6 +20,13 @@ type signedYamlFile struct { } `yaml:"vars"` } +var verificationCommand = "insights-client" + +var verificationArgs = []string{ + "-m", "insights.client.apps.ansible.playbook_verifier", + "--quiet", "--payload", "noop", "--content-type", "noop", +} + // Verify that no one tampered with yaml file func verifyYamlFile(yamlData []byte) bool { @@ -31,18 +38,14 @@ func verifyYamlFile(yamlData []byte) bool { // --payload here will be a no-op because no upload is performed when using the verifier // but, it will allow us to update the egg! - args := []string{ - "-m", "insights.client.apps.ansible.playbook_verifier", - "--quiet", "--payload", "noop", "--content-type", "noop", - } env := os.Environ() if !*config.InsightsCoreGPGCheck { - args = append(args, "--no-gpg") + verificationArgs = append(verificationArgs, "--no-gpg") env = append(env, "BYPASS_GPG=True") } - cmd := exec.Command("insights-client", args...) + cmd := exec.Command(verificationCommand, verificationArgs...) cmd.Env = env stdin, err := cmd.StdinPipe() if err != nil { diff --git a/src/runner_test.go b/src/runner_test.go index 8c85fef..3507e9d 100644 --- a/src/runner_test.go +++ b/src/runner_test.go @@ -74,38 +74,47 @@ vars: func TestVerifyYamlFile(t *testing.T) { testCases := []struct { - name string - verifyYAML bool - yamlData []byte - expectedResult bool + name string + yamlData []byte + verifyYAML bool + verificationCommand string + verificationArgs []string + shouldDoInsightsCoreGPGCheck bool + expectedResult bool }{ { - name: "verification disabled", - verifyYAML: false, - yamlData: []byte{}, - expectedResult: true, + name: "verification disabled", + verifyYAML: false, + yamlData: []byte{}, + shouldDoInsightsCoreGPGCheck: false, + expectedResult: true, }, - // FIXME: This should succedd but now verification fails on missing insighs-client - // We also need valid signature { - name: "verification enabled and verification succeeds", - verifyYAML: true, - yamlData: []byte("valid-yaml"), - expectedResult: false, + name: "verification enabled and verification succeeds", + verifyYAML: true, + yamlData: []byte("valid-yaml"), + verificationCommand: "true", + verificationArgs: []string{}, + shouldDoInsightsCoreGPGCheck: false, + expectedResult: true, }, - // FIXME: Valid test case but fails because of missing insights-client { - name: "verification is enabled and verification fails", - verifyYAML: true, - yamlData: []byte("invalid-yaml"), - expectedResult: false, + name: "verification is enabled and verification fails", + verifyYAML: true, + yamlData: []byte("invalid-yaml"), + verificationCommand: "false", + verificationArgs: []string{}, + shouldDoInsightsCoreGPGCheck: false, + expectedResult: false, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { shouldVerifyYaml := tc.verifyYAML - shouldDoInsightsCoreGPGCheck := false // Reset to false for each test case + shouldDoInsightsCoreGPGCheck := tc.shouldDoInsightsCoreGPGCheck + verificationCommand = tc.verificationCommand + verificationArgs = tc.verificationArgs config = &Config{ VerifyYAML: &shouldVerifyYaml,