From c340a308b94fbc42c1abf6dab3afbdbf42c0ab40 Mon Sep 17 00:00:00 2001 From: David-Jaeyoon-Lee Date: Mon, 22 Jul 2024 08:36:19 +0000 Subject: [PATCH 1/3] Shorten message for crashOnFailureFetchingExpectations flag Signed-off-by: David-Jaeyoon-Lee --- pkg/readiness/ready_tracker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index 4c806b888f9..ec65e092db5 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -46,7 +46,7 @@ import ( var log = logf.Log.WithName("readiness-tracker") -var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors that occur when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy by serving before it has loaded in all policies. Enabling this will help prevent under-enforcement at the risk of crashing during startup for issues like network errors. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations are set to retry until success so failures during fetching expectations currently do not occur.") +var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy. Enabling this will help prevent under-enforcement at the risk of crashing during startup. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations will retry until success.") const ( constraintGroup = "constraints.gatekeeper.sh" From 003ec99fceb25d8abebdd733c4d86dce7c696628 Mon Sep 17 00:00:00 2001 From: David-Jaeyoon-Lee Date: Tue, 23 Jul 2024 22:26:18 +0000 Subject: [PATCH 2/3] Change flag to be commented out and replaced with a false constant The value of the flag is moot so we want to hide it from the user. Signed-off-by: David-Jaeyoon-Lee --- pkg/readiness/ready_tracker.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index ec65e092db5..ad329843e40 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -17,7 +17,6 @@ package readiness import ( "context" - "flag" "fmt" "net/http" "sync" @@ -46,7 +45,9 @@ import ( var log = logf.Log.WithName("readiness-tracker") -var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy. Enabling this will help prevent under-enforcement at the risk of crashing during startup. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations will retry until success.") +// Commenting out the flag and replacing with a false boolean constant because the value of the flag is currently moot without a retry limit +// var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy. Enabling this will help prevent under-enforcement at the risk of crashing during startup. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations will retry until success.") +const crashOnFailureFetchingExpectations = false const ( constraintGroup = "constraints.gatekeeper.sh" @@ -90,7 +91,8 @@ type Tracker struct { // NewTracker creates a new Tracker and initializes the internal trackers. func NewTracker(lister Lister, mutationEnabled, externalDataEnabled, expansionEnabled bool) *Tracker { - return newTracker(lister, mutationEnabled, externalDataEnabled, expansionEnabled, *crashOnFailureFetchingExpectations, nil, nil) + // Dereference crashOnFailureFetchingExpectations when we change crashOnFailureFetchingExpectations back to a flag + return newTracker(lister, mutationEnabled, externalDataEnabled, expansionEnabled, crashOnFailureFetchingExpectations, nil, nil) } func newTracker(lister Lister, mutationEnabled, externalDataEnabled, expansionEnabled bool, crashOnFailure bool, trackListerPredicateOverride retryPredicate, fn objDataFactory) *Tracker { From 2a18f78a313a28d1594e8bba26dde5bfa7b17363 Mon Sep 17 00:00:00 2001 From: David-Jaeyoon-Lee Date: Wed, 24 Jul 2024 13:06:01 +0000 Subject: [PATCH 3/3] Add TODOs on crashOnFailureFetchingExpectations When we support retry count on fetching expectations we want to expose the crashOnFailureFetchingExpectations as a flag instead of a constant. Signed-off-by: David-Jaeyoon-Lee --- pkg/readiness/ready_tracker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index ad329843e40..36a1ce7958c 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -45,7 +45,7 @@ import ( var log = logf.Log.WithName("readiness-tracker") -// Commenting out the flag and replacing with a false boolean constant because the value of the flag is currently moot without a retry limit +// TODO: Uncomment the flag and deleted the boolean constant when we support retry limits (currently the value of the flag is moot without a retry limit since failure won't happen due to unlimited retries) // var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy. Enabling this will help prevent under-enforcement at the risk of crashing during startup. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations will retry until success.") const crashOnFailureFetchingExpectations = false @@ -91,7 +91,7 @@ type Tracker struct { // NewTracker creates a new Tracker and initializes the internal trackers. func NewTracker(lister Lister, mutationEnabled, externalDataEnabled, expansionEnabled bool) *Tracker { - // Dereference crashOnFailureFetchingExpectations when we change crashOnFailureFetchingExpectations back to a flag + // TODO: Dereference crashOnFailureFetchingExpectations when we change crashOnFailureFetchingExpectations to a flag return newTracker(lister, mutationEnabled, externalDataEnabled, expansionEnabled, crashOnFailureFetchingExpectations, nil, nil) }