mirror of
https://github.com/stakater/Reloader.git
synced 2026-05-06 00:36:39 +00:00
fix: Controller not respecting ignore* flags
This commit is contained in:
Pradeep Lakshmi Narasimha
committed by
Felix
Felix
parent
b8edc25177
commit
dd0807e951
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/sirupsen/logrus"
|
||||
"github.com/stakater/Reloader/internal/pkg/constants"
|
||||
"github.com/stakater/Reloader/internal/pkg/options"
|
||||
"github.com/stakater/Reloader/internal/pkg/util"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/labels"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
@@ -74,6 +75,8 @@ type ReloaderOptions struct {
|
||||
WebhookUrl string `json:"webhookUrl"`
|
||||
// ResourcesToIgnore is a list of resource types to ignore (e.g., "configmaps" or "secrets")
|
||||
ResourcesToIgnore []string `json:"resourcesToIgnore"`
|
||||
// WorkloadTypesToIgnore is a list of workload types to ignore (e.g., "jobs" or "cronjobs")
|
||||
WorkloadTypesToIgnore []string `json:"workloadTypesToIgnore"`
|
||||
// NamespaceSelectors is a list of label selectors to filter namespaces to watch
|
||||
NamespaceSelectors []string `json:"namespaceSelectors"`
|
||||
// ResourceSelectors is a list of label selectors to filter ConfigMaps and Secrets to watch
|
||||
@@ -182,6 +185,32 @@ func GetResourceLabelSelector(slice []string) (string, error) {
|
||||
// ShouldReload checks if a resource should be reloaded based on its annotations and the provided options.
|
||||
func ShouldReload(config Config, resourceType string, annotations Map, podAnnotations Map, options *ReloaderOptions) ReloadCheckResult {
|
||||
|
||||
// Check if this workload type should be ignored
|
||||
if len(options.WorkloadTypesToIgnore) > 0 {
|
||||
ignoredWorkloadTypes, err := util.GetIgnoredWorkloadTypesList()
|
||||
if err != nil {
|
||||
logrus.Errorf("Failed to parse ignored workload types: %v", err)
|
||||
} else {
|
||||
// Map Kubernetes resource types to CLI-friendly names for comparison
|
||||
var resourceToCheck string
|
||||
switch resourceType {
|
||||
case "Job":
|
||||
resourceToCheck = "jobs"
|
||||
case "CronJob":
|
||||
resourceToCheck = "cronjobs"
|
||||
default:
|
||||
resourceToCheck = resourceType // For other types, use as-is
|
||||
}
|
||||
|
||||
// Check if current resource type should be ignored
|
||||
if ignoredWorkloadTypes.Contains(resourceToCheck) {
|
||||
return ReloadCheckResult{
|
||||
ShouldReload: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ignoreResourceAnnotatonValue := config.ResourceAnnotations[options.IgnoreResourceAnnotation]
|
||||
if ignoreResourceAnnotatonValue == "true" {
|
||||
return ReloadCheckResult{
|
||||
@@ -304,6 +333,7 @@ func GetCommandLineOptions() *ReloaderOptions {
|
||||
CommandLineOptions.EnableHA = options.EnableHA
|
||||
CommandLineOptions.WebhookUrl = options.WebhookUrl
|
||||
CommandLineOptions.ResourcesToIgnore = options.ResourcesToIgnore
|
||||
CommandLineOptions.WorkloadTypesToIgnore = options.WorkloadTypesToIgnore
|
||||
CommandLineOptions.NamespaceSelectors = options.NamespaceSelectors
|
||||
CommandLineOptions.ResourceSelectors = options.ResourceSelectors
|
||||
CommandLineOptions.NamespacesToIgnore = options.NamespacesToIgnore
|
||||
|
||||
224
pkg/common/common_test.go
Normal file
224
pkg/common/common_test.go
Normal file
@@ -0,0 +1,224 @@
|
||||
package common
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stakater/Reloader/internal/pkg/options"
|
||||
)
|
||||
|
||||
func TestShouldReload_IgnoredWorkloadTypes(t *testing.T) {
|
||||
// Save original state
|
||||
originalWorkloadTypes := options.WorkloadTypesToIgnore
|
||||
defer func() {
|
||||
options.WorkloadTypesToIgnore = originalWorkloadTypes
|
||||
}()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
ignoredWorkloadTypes []string
|
||||
resourceType string
|
||||
shouldReload bool
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "Jobs ignored - Job should not reload",
|
||||
ignoredWorkloadTypes: []string{"jobs"},
|
||||
resourceType: "Job",
|
||||
shouldReload: false,
|
||||
description: "When jobs are ignored, Job resources should not be reloaded",
|
||||
},
|
||||
{
|
||||
name: "Jobs ignored - CronJob should reload",
|
||||
ignoredWorkloadTypes: []string{"jobs"},
|
||||
resourceType: "CronJob",
|
||||
shouldReload: true,
|
||||
description: "When jobs are ignored, CronJob resources should still be processed",
|
||||
},
|
||||
{
|
||||
name: "CronJobs ignored - CronJob should not reload",
|
||||
ignoredWorkloadTypes: []string{"cronjobs"},
|
||||
resourceType: "CronJob",
|
||||
shouldReload: false,
|
||||
description: "When cronjobs are ignored, CronJob resources should not be reloaded",
|
||||
},
|
||||
{
|
||||
name: "CronJobs ignored - Job should reload",
|
||||
ignoredWorkloadTypes: []string{"cronjobs"},
|
||||
resourceType: "Job",
|
||||
shouldReload: true,
|
||||
description: "When cronjobs are ignored, Job resources should still be processed",
|
||||
},
|
||||
{
|
||||
name: "Both ignored - Job should not reload",
|
||||
ignoredWorkloadTypes: []string{"jobs", "cronjobs"},
|
||||
resourceType: "Job",
|
||||
shouldReload: false,
|
||||
description: "When both are ignored, Job resources should not be reloaded",
|
||||
},
|
||||
{
|
||||
name: "Both ignored - CronJob should not reload",
|
||||
ignoredWorkloadTypes: []string{"jobs", "cronjobs"},
|
||||
resourceType: "CronJob",
|
||||
shouldReload: false,
|
||||
description: "When both are ignored, CronJob resources should not be reloaded",
|
||||
},
|
||||
{
|
||||
name: "Both ignored - Deployment should reload",
|
||||
ignoredWorkloadTypes: []string{"jobs", "cronjobs"},
|
||||
resourceType: "Deployment",
|
||||
shouldReload: true,
|
||||
description: "When both are ignored, other workload types should still be processed",
|
||||
},
|
||||
{
|
||||
name: "None ignored - Job should reload",
|
||||
ignoredWorkloadTypes: []string{},
|
||||
resourceType: "Job",
|
||||
shouldReload: true,
|
||||
description: "When nothing is ignored, all workload types should be processed",
|
||||
},
|
||||
{
|
||||
name: "None ignored - CronJob should reload",
|
||||
ignoredWorkloadTypes: []string{},
|
||||
resourceType: "CronJob",
|
||||
shouldReload: true,
|
||||
description: "When nothing is ignored, all workload types should be processed",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Set the ignored workload types
|
||||
options.WorkloadTypesToIgnore = tt.ignoredWorkloadTypes
|
||||
|
||||
// Create minimal test config and options
|
||||
config := Config{
|
||||
ResourceName: "test-resource",
|
||||
Annotation: "configmap.reloader.stakater.com/reload",
|
||||
}
|
||||
|
||||
annotations := Map{
|
||||
"configmap.reloader.stakater.com/reload": "test-config",
|
||||
}
|
||||
|
||||
// Create ReloaderOptions with the ignored workload types
|
||||
opts := &ReloaderOptions{
|
||||
WorkloadTypesToIgnore: tt.ignoredWorkloadTypes,
|
||||
AutoReloadAll: true, // Enable auto-reload to simplify test
|
||||
ReloaderAutoAnnotation: "reloader.stakater.com/auto",
|
||||
}
|
||||
|
||||
// Call ShouldReload
|
||||
result := ShouldReload(config, tt.resourceType, annotations, Map{}, opts)
|
||||
|
||||
// Check the result
|
||||
if result.ShouldReload != tt.shouldReload {
|
||||
t.Errorf("For resource type %s with ignored types %v, expected ShouldReload=%v, got=%v",
|
||||
tt.resourceType, tt.ignoredWorkloadTypes, tt.shouldReload, result.ShouldReload)
|
||||
}
|
||||
|
||||
t.Logf("✓ %s", tt.description)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldReload_IgnoredWorkloadTypes_ValidationError(t *testing.T) {
|
||||
// Save original state
|
||||
originalWorkloadTypes := options.WorkloadTypesToIgnore
|
||||
defer func() {
|
||||
options.WorkloadTypesToIgnore = originalWorkloadTypes
|
||||
}()
|
||||
|
||||
// Test with invalid workload type - should still continue processing
|
||||
options.WorkloadTypesToIgnore = []string{"invalid"}
|
||||
|
||||
config := Config{
|
||||
ResourceName: "test-resource",
|
||||
Annotation: "configmap.reloader.stakater.com/reload",
|
||||
}
|
||||
|
||||
annotations := Map{
|
||||
"configmap.reloader.stakater.com/reload": "test-config",
|
||||
}
|
||||
|
||||
opts := &ReloaderOptions{
|
||||
WorkloadTypesToIgnore: []string{"invalid"},
|
||||
AutoReloadAll: true, // Enable auto-reload to simplify test
|
||||
ReloaderAutoAnnotation: "reloader.stakater.com/auto",
|
||||
}
|
||||
|
||||
// Should not panic and should continue with normal processing
|
||||
result := ShouldReload(config, "Job", annotations, Map{}, opts)
|
||||
|
||||
// Since validation failed, it should continue with normal processing (should reload)
|
||||
if !result.ShouldReload {
|
||||
t.Errorf("Expected ShouldReload=true when validation fails, got=%v", result.ShouldReload)
|
||||
}
|
||||
}
|
||||
|
||||
// Test that validates the fix for issue #996
|
||||
func TestShouldReload_IssueRBACPermissionFixed(t *testing.T) {
|
||||
// Save original state
|
||||
originalWorkloadTypes := options.WorkloadTypesToIgnore
|
||||
defer func() {
|
||||
options.WorkloadTypesToIgnore = originalWorkloadTypes
|
||||
}()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
ignoredWorkloadTypes []string
|
||||
resourceType string
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "Issue #996 - ignoreJobs prevents Job processing",
|
||||
ignoredWorkloadTypes: []string{"jobs"},
|
||||
resourceType: "Job",
|
||||
description: "Job resources are skipped entirely, preventing RBAC permission errors",
|
||||
},
|
||||
{
|
||||
name: "Issue #996 - ignoreCronJobs prevents CronJob processing",
|
||||
ignoredWorkloadTypes: []string{"cronjobs"},
|
||||
resourceType: "CronJob",
|
||||
description: "CronJob resources are skipped entirely, preventing RBAC permission errors",
|
||||
},
|
||||
{
|
||||
name: "Issue #996 - both ignored prevent both types",
|
||||
ignoredWorkloadTypes: []string{"jobs", "cronjobs"},
|
||||
resourceType: "Job",
|
||||
description: "Job resources are skipped entirely when both types are ignored",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Set the ignored workload types
|
||||
options.WorkloadTypesToIgnore = tt.ignoredWorkloadTypes
|
||||
|
||||
config := Config{
|
||||
ResourceName: "test-resource",
|
||||
Annotation: "configmap.reloader.stakater.com/reload",
|
||||
}
|
||||
|
||||
annotations := Map{
|
||||
"configmap.reloader.stakater.com/reload": "test-config",
|
||||
}
|
||||
|
||||
opts := &ReloaderOptions{
|
||||
WorkloadTypesToIgnore: tt.ignoredWorkloadTypes,
|
||||
AutoReloadAll: true, // Enable auto-reload to simplify test
|
||||
ReloaderAutoAnnotation: "reloader.stakater.com/auto",
|
||||
}
|
||||
|
||||
// Call ShouldReload
|
||||
result := ShouldReload(config, tt.resourceType, annotations, Map{}, opts)
|
||||
|
||||
// Should not reload when workload type is ignored
|
||||
if result.ShouldReload {
|
||||
t.Errorf("Expected ShouldReload=false for ignored workload type %s, got=%v",
|
||||
tt.resourceType, result.ShouldReload)
|
||||
}
|
||||
|
||||
t.Logf("✓ %s", tt.description)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user