Merge pull request #104 from andywaltlova/fix/verify-true-gpg-false

Remove insights_core_gpg_check from worker config

README.md

@@ -135,9 +135,6 @@ directive: "rhc-worker-script"
 # whether to verify incoming yaml files
 verify_yaml: true
 
-# perform the insights-client GPG check on the insights-core egg
-insights_core_gpg_check: true
-
 # temporary directory in which the temporary files with executed bash scripts are created
 temporary_worker_directory: "/var/lib/rhc-worker-script"
 ```

packaging/rhc-worker-script.spec

@@ -72,9 +72,6 @@ directive: "%{name}"
 # whether to verify incoming yaml files
 verify_yaml: true
 
-# perform the insights-client GPG check on the insights-core egg
-insights_core_gpg_check: true
-
 # temporary directory in which the temporary script will be placed and executed.
 temporary_worker_directory: "/var/lib/rhc-worker-script"
 EOF

rhc-worker-script.yml

@@ -2,10 +2,7 @@
 directive: "rhc-worker-script"
 
 # whether to verify incoming yaml files
-verify_yaml: false
-
-# perform the insights-client GPG check on the insights-core egg
-insights_core_gpg_check: false
+verify_yaml: true
 
 # temporary directory in which the temporary script will be placed and executed.
 temporary_worker_directory: "/var/lib/rhc-worker-script"

src/runner.go

@@ -41,15 +41,7 @@ func verifyYamlFile(yamlData []byte) bool {
 	}
 
 	env := os.Environ()
-	if !*config.InsightsCoreGPGCheck {
-		log.Infoln("Calling insights-client with --no-gpg to skip signature validation...")
-		// --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!
-		verificationArgs = append(verificationArgs, "--no-gpg")
-		env = append(env, "BYPASS_GPG=True")
-	} else {
-		log.Infoln("Calling insights-client with gpg signature validation...")
-	}
+	log.Infoln("Calling insights-client playbook verifier ...")
 
 	cmd := exec.Command(verificationCommand, verificationArgs...)
 	cmd.Env = env
@@ -76,11 +68,7 @@ func verifyYamlFile(yamlData []byte) bool {
 		return false
 	}
 
-	if !*config.InsightsCoreGPGCheck {
-		log.Infoln("GPG verification is disabled and thus considered as valid")
-	} else {
-		log.Infoln("Signature of yaml file is valid")
-	}
+	log.Infoln("Signature of yaml file is valid")
 	return true
 }
 

src/runner_test.go

@@ -36,12 +36,10 @@ func TestProcessSignedScript(t *testing.T) {
 	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)
@@ -56,51 +54,45 @@ func TestProcessSignedScript(t *testing.T) {
 
 func TestVerifyYamlFile(t *testing.T) {
 	testCases := []struct {
-		name                         string
-		yamlData                     []byte
-		verifyYAML                   bool
-		verificationCommand          string
-		verificationArgs             []string
-		shouldDoInsightsCoreGPGCheck bool
-		expectedResult               bool
+		name                string
+		yamlData            []byte
+		verifyYAML          bool
+		verificationCommand string
+		verificationArgs    []string
+		expectedResult      bool
 	}{
 		{
-			name:                         "verification disabled",
-			verifyYAML:                   false,
-			yamlData:                     ExampleYamlData,
-			shouldDoInsightsCoreGPGCheck: false,
-			expectedResult:               true,
+			name:           "verification disabled",
+			verifyYAML:     false,
+			yamlData:       ExampleYamlData,
+			expectedResult: true,
 		},
 		{
-			name:                         "verification enabled and verification succeeds",
-			verifyYAML:                   true,
-			yamlData:                     ExampleYamlData,
-			verificationCommand:          "true",
-			verificationArgs:             []string{},
-			shouldDoInsightsCoreGPGCheck: false,
-			expectedResult:               true,
+			name:                "verification enabled and verification succeeds",
+			verifyYAML:          true,
+			yamlData:            ExampleYamlData,
+			verificationCommand: "true",
+			verificationArgs:    []string{},
+			expectedResult:      true,
 		},
 		{
-			name:                         "verification is enabled and verification fails",
-			verifyYAML:                   true,
-			yamlData:                     ExampleYamlData,
-			verificationCommand:          "false",
-			verificationArgs:             []string{},
-			shouldDoInsightsCoreGPGCheck: false,
-			expectedResult:               false,
+			name:                "verification is enabled and verification fails",
+			verifyYAML:          true,
+			yamlData:            ExampleYamlData,
+			verificationCommand: "false",
+			verificationArgs:    []string{},
+			expectedResult:      false,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			shouldVerifyYaml := tc.verifyYAML
-			shouldDoInsightsCoreGPGCheck := tc.shouldDoInsightsCoreGPGCheck
 			verificationCommand = tc.verificationCommand
 			verificationArgs = tc.verificationArgs
 
 			config = &Config{
-				VerifyYAML:           &shouldVerifyYaml,
-				InsightsCoreGPGCheck: &shouldDoInsightsCoreGPGCheck,
+				VerifyYAML: &shouldVerifyYaml,
 			}
 
 			result := verifyYamlFile(tc.yamlData)

src/server_test.go

@@ -113,12 +113,10 @@ func TestProcessData(t *testing.T) {
 	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"

src/util.go

@@ -97,7 +97,6 @@ func constructMetadata(receivedMetadata map[string]string, contentType string) m
 type Config struct {
 	Directive                *string `yaml:"directive,omitempty"`
 	VerifyYAML               *bool   `yaml:"verify_yaml,omitempty"`
-	InsightsCoreGPGCheck     *bool   `yaml:"insights_core_gpg_check,omitempty"`
 	TemporaryWorkerDirectory *string `yaml:"temporary_worker_directory,omitempty"`
 }
 
@@ -116,12 +115,6 @@ func setDefaultValues(config *Config) {
 		config.VerifyYAML = &defaultVerifyYamlValue
 	}
 
-	if config.InsightsCoreGPGCheck == nil {
-		defaultGpgCheckValue := true
-		log.Infof("config 'insights_core_gpg_check' value is empty default value (%t) will be used", defaultGpgCheckValue)
-		config.InsightsCoreGPGCheck = &defaultGpgCheckValue
-	}
-
 	if config.TemporaryWorkerDirectory == nil {
 		defaultTemporaryWorkerDirectoryValue := "/var/lib/rhc-worker-script"
 		log.Infof("config 'temporary_worker_directory' value is empty default value (%s) will be used", defaultTemporaryWorkerDirectoryValue)
@@ -146,9 +139,6 @@ func loadYAMLConfig(filePath string) *Config {
 }
 
 // Load config from given filepath, if config doesn't exist then default config values are used
-// Directive = rhc-worker-script
-// VerifyYAML = "1"
-// InsightsCoreGPGCheck = "1"
 func loadConfigOrDefault(filePath string) *Config {
 	config := &Config{}
 	_, err := os.Stat(filePath)

src/util_test.go

@@ -142,7 +142,6 @@ func createTempYAMLFile(content string) (string, error) {
 func compareConfigs(c1, c2 *Config) bool {
 	return *c1.Directive == *c2.Directive &&
 		*c1.VerifyYAML == *c2.VerifyYAML &&
-		*c1.InsightsCoreGPGCheck == *c2.InsightsCoreGPGCheck &&
 		*c1.TemporaryWorkerDirectory == *c2.TemporaryWorkerDirectory
 }
 
@@ -160,7 +159,6 @@ const validYAMLData = `
 directive: "rhc-worker-script"
 verify_yaml: true
 verify_yaml_version_check: true
-insights_core_gpg_check: true
 temporary_worker_directory: "/var/lib/rhc-worker-script"
 `
 
@@ -172,7 +170,6 @@ func TestLoadConfigOrDefault(t *testing.T) {
 	expectedConfig := &Config{
 		Directive:                strPtr("rhc-worker-script"),
 		VerifyYAML:               boolPtr(true),
-		InsightsCoreGPGCheck:     boolPtr(true),
 		TemporaryWorkerDirectory: strPtr("/var/lib/rhc-worker-script"),
 	}
 
@@ -239,13 +236,13 @@ func TestSetDefaultValues(t *testing.T) {
 	}{
 		{
 			name: "test default values",
-			args: args{config: &Config{nil, nil, nil, nil}},
-			want: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
+			args: args{config: &Config{nil, nil, nil}},
+			want: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
 		},
 		{
 			name: "test non default values",
-			args: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
-			want: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
+			args: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
+			want: args{config: &Config{strPtr("rhc-worker-script"), boolPtr(true), strPtr("/var/lib/rhc-worker-script")}},
 		},
 	}
 	for _, tt := range tests {