Skip to content

Conversation

@praddy26
Copy link
Contributor

@praddy26 praddy26 commented Aug 18, 2025

Problem

Fixes #996: Setting ignoreJobs or ignoreCronJobs in Helm values removes RBAC permissions but Reloader still attempts to access Jobs/CronJobs, causing permission errors.

Solution

Implemented a unified --ignored-workload-types flag that accepts both jobs and cronjobs values (individually or together), replacing the need for separate boolean flags while maintaining backward compatibility.

Key Changes

Core Implementation

  • Added new flag: --ignored-workload-types (StringSliceVar) accepting jobs, cronjobs, or both
  • Enhanced validation: Added GetIgnoredWorkloadTypesList() function with clear error messages
  • Updated configuration: Extended ReloaderOptions struct with WorkloadTypesToIgnore field

Handler Logic

  • Modified rolling upgrade logic: Updated ShouldReload to check ignored workload types before processing Jobs/CronJobs
  • Conditional processing: Skip workload processing when types are in the ignored list

Helm Integration

  • Updated deployment template: Added conditional logic to generate --ignored-workload-types arguments based on existing ignoreJobs/ignoreCronJobs values
  • Backward compatibility: Existing values.yaml settings continue to work seamlessly

Testing

  • Comprehensive test coverage: Added validation for all input combinations
  • Integration tests: Verified flag parsing and business logic
  • Build validation: Ensured no regressions in existing functionality

Usage Examples

Command Line

# Ignore only Jobs
--ignored-workload-types=jobs

# Ignore only CronJobs  
--ignored-workload-types=cronjobs

# Ignore both
--ignored-workload-types=jobs,cronjobs

@praddy26 praddy26 force-pushed the fix-bug-996 branch 2 times, most recently from 70f3783 to 92590e9 Compare August 18, 2025 14:17
@Felix-Stakater
Copy link
Contributor

Hi! I think this functionality is better suited inside the ShouldReload. From what i can tell we have all the context from the cli args + the resource being referenced so it should be possible to do the check from there.

@praddy26 praddy26 force-pushed the fix-bug-996 branch 3 times, most recently from 3d013e6 to 8508481 Compare August 22, 2025 13:10
Copy link
Contributor

@Felix-Stakater Felix-Stakater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for your contribution!

@Felix-Stakater Felix-Stakater merged commit dd0807e into stakater:master Aug 29, 2025
12 checks passed
@ryndaniels
Copy link

@praddy26 just an FYI - we are running with chart v2.2.3 (which looks like it should have this fix) and we're still getting the permission errors in the logs when ignoreJobs/ignoreCronJobs are set in the chart. Not sure if anyone else is still seeing this as well

@r4j4h
Copy link

r4j4h commented Sep 24, 2025

Can confirm with 2.2.3 we are also seeing this behavior

@praddy26
Copy link
Contributor Author

@ryndaniels @r4j4h Thanks for flagging this. I will work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] setting ignoreJobs/ignoreCronJobs just removes the RBAC permissions

4 participants