Skip to content

Commit

Permalink
UPSTREAM: <carry>: sets X-OpenShift-Internal-If-Not-Ready HTTP Header…
Browse files Browse the repository at this point in the history
… for GC and Namespace controllers

In general, setting the header will result in getting 429 when the server hasn't been ready.
This prevents certain controllers like GC, Namespace from accidentally removing resources when the caches haven't been fully synchronized.

OpenShift-Rebase-Source: 2ebf199
  • Loading branch information
p0lyn0mial authored and soltysh committed Dec 8, 2023
1 parent a5d9180 commit 8e695e7
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 2 deletions.
1 change: 1 addition & 0 deletions cmd/kube-controller-manager/app/config/patch.go
Expand Up @@ -15,4 +15,5 @@ type OpenShiftContext struct {
UnsupportedKubeAPIOverPreferredHost bool
PreferredHostRoundTripperWrapperFn transport.WrapperFunc
PreferredHostHealthMonitor *health.Prober
CustomRoundTrippers []transport.WrapperFunc
}
2 changes: 1 addition & 1 deletion cmd/kube-controller-manager/app/controllermanager.go
Expand Up @@ -137,7 +137,7 @@ controller, and serviceaccounts controller.`,
}
cliflag.PrintFlags(cmd.Flags())

if err := SetUpPreferredHostForOpenShift(s); err != nil {
if err := SetUpCustomRoundTrippersForOpenShift(s); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/kube-controller-manager/app/options/options.go
Expand Up @@ -456,6 +456,9 @@ func (s KubeControllerManagerOptions) Config(allControllers []string, disabledBy
libgorestclient.DefaultServerName(kubeconfig)
kubeconfig.Wrap(s.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
}
for _, customOpenShiftRoundTripper := range s.OpenShiftContext.CustomRoundTrippers {
kubeconfig.Wrap(customOpenShiftRoundTripper)
}

client, err := clientset.NewForConfig(restclient.AddUserAgent(kubeconfig, KubeControllerManagerUserAgent))
if err != nil {
Expand Down
33 changes: 32 additions & 1 deletion cmd/kube-controller-manager/app/patch.go
Expand Up @@ -3,14 +3,17 @@ package app
import (
"fmt"
"io/ioutil"
"net/http"
"path"
"strings"
"time"

"k8s.io/apimachinery/pkg/util/json"
kyaml "k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/client-go/informers"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/transport"
"k8s.io/component-base/metrics/legacyregistry"
"k8s.io/kubernetes/cmd/kube-controller-manager/app/config"
"k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
Expand All @@ -21,7 +24,9 @@ import (

var InformerFactoryOverride informers.SharedInformerFactory

func SetUpPreferredHostForOpenShift(controllerManagerOptions *options.KubeControllerManagerOptions) error {
func SetUpCustomRoundTrippersForOpenShift(controllerManagerOptions *options.KubeControllerManagerOptions) error {
controllerManagerOptions.OpenShiftContext.CustomRoundTrippers = []transport.WrapperFunc{newRejectIfNotReadyHeaderRoundTripper([]string{"generic-garbage-collector", "namespace-controller"})}

if !controllerManagerOptions.OpenShiftContext.UnsupportedKubeAPIOverPreferredHost {
return nil
}
Expand Down Expand Up @@ -54,6 +59,7 @@ func SetUpPreferredHostForOpenShift(controllerManagerOptions *options.KubeContro

controllerManagerOptions.Authentication.WithCustomRoundTripper(controllerManagerOptions.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
controllerManagerOptions.Authorization.WithCustomRoundTripper(controllerManagerOptions.OpenShiftContext.PreferredHostRoundTripperWrapperFn)

return nil
}

Expand Down Expand Up @@ -133,3 +139,28 @@ func createRestConfigForHealthMonitor(restConfig *rest.Config) *rest.Config {

return &restConfigCopy
}

// newRejectIfNotReadyHeaderRoundTripper a middleware for setting X-OpenShift-Internal-If-Not-Ready HTTP Header for the given users.
// In general, setting the header will result in getting 429 when the server hasn't been ready.
// This prevents certain controllers like GC, Namespace from accidentally removing resources when the caches haven't been fully synchronized.
func newRejectIfNotReadyHeaderRoundTripper(eligibleUsers []string) func(http.RoundTripper) http.RoundTripper {
return func(rt http.RoundTripper) http.RoundTripper {
return &rejectIfNotReadyHeaderRT{baseRT: rt, eligibleUsers: eligibleUsers}
}
}

type rejectIfNotReadyHeaderRT struct {
baseRT http.RoundTripper
eligibleUsers []string
}

func (rt *rejectIfNotReadyHeaderRT) RoundTrip(r *http.Request) (*http.Response, error) {
currentUser := r.UserAgent()
for _, eligibleUser := range rt.eligibleUsers {
if strings.Contains(currentUser, eligibleUser) {
r.Header.Set("X-OpenShift-Internal-If-Not-Ready", "reject")
break
}
}
return rt.baseRT.RoundTrip(r)
}
74 changes: 74 additions & 0 deletions cmd/kube-controller-manager/app/patch_test.go
@@ -0,0 +1,74 @@
package app

import (
"fmt"
"net/http"
"net/textproto"
"testing"
)

func TestRejectIfNotReadyHeaderRT(t *testing.T) {
scenarios := []struct {
name string
eligibleUsers []string
currentUser string
expectHeader bool
}{
{
name: "scenario 1: happy path",
currentUser: "system:serviceaccount:kube-system:generic-garbage-collector",
eligibleUsers: []string{"generic-garbage-collector", "namespace-controller"},
expectHeader: true,
},
{
name: "scenario 2: ineligible user",
currentUser: "system:serviceaccount:kube-system:service-account-controller",
eligibleUsers: []string{"generic-garbage-collector", "namespace-controller"},
expectHeader: false,
},
}

for _, scenario := range scenarios {
t.Run(scenario.name, func(t *testing.T) {
// set up the test
fakeRT := fakeRTFunc(func(r *http.Request) (*http.Response, error) {
// this is where we validate if the header was set or not
headerSet := func() bool {
if len(r.Header.Get("X-OpenShift-Internal-If-Not-Ready")) > 0 {
return true
}
return false
}()
if scenario.expectHeader && !headerSet {
return nil, fmt.Errorf("%v header wasn't set", textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
}
if !scenario.expectHeader && headerSet {
return nil, fmt.Errorf("didn't expect %v header", textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
}
if scenario.expectHeader {
if value := r.Header.Get("X-OpenShift-Internal-If-Not-Ready"); value != "reject" {
return nil, fmt.Errorf("unexpected value %v in the %v header, expected \"reject\"", value, textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
}
}
return nil, nil
})
target := newRejectIfNotReadyHeaderRoundTripper(scenario.eligibleUsers)(fakeRT)
req, err := http.NewRequest("GET", "", nil)
if err != nil {
t.Fatal(err)
}
req.Header.Set("User-Agent", scenario.currentUser)

// act and validate
if _, err := target.RoundTrip(req); err != nil {
t.Fatal(err)
}
})
}
}

type fakeRTFunc func(r *http.Request) (*http.Response, error)

func (rt fakeRTFunc) RoundTrip(r *http.Request) (*http.Response, error) {
return rt(r)
}

0 comments on commit 8e695e7

Please sign in to comment.