Skip to content

Commit 2f5cd7c

Browse files
p0lyn0mialbertinatto
authored andcommitted
UPSTREAM: <carry>: sets X-OpenShift-Internal-If-Not-Ready HTTP Header 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
1 parent 426b917 commit 2f5cd7c

File tree

5 files changed

+111
-2
lines changed

5 files changed

+111
-2
lines changed

cmd/kube-controller-manager/app/config/patch.go

+1
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,5 @@ type OpenShiftContext struct {
1515
UnsupportedKubeAPIOverPreferredHost bool
1616
PreferredHostRoundTripperWrapperFn transport.WrapperFunc
1717
PreferredHostHealthMonitor *health.Prober
18+
CustomRoundTrippers []transport.WrapperFunc
1819
}

cmd/kube-controller-manager/app/controllermanager.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ controller, and serviceaccounts controller.`,
134134
}
135135
cliflag.PrintFlags(cmd.Flags())
136136

137-
if err := SetUpPreferredHostForOpenShift(s); err != nil {
137+
if err := SetUpCustomRoundTrippersForOpenShift(s); err != nil {
138138
fmt.Fprintf(os.Stderr, "%v\n", err)
139139
os.Exit(1)
140140
}

cmd/kube-controller-manager/app/options/options.go

+3
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,9 @@ func (s KubeControllerManagerOptions) Config(allControllers []string, disabledBy
502502
libgorestclient.DefaultServerName(kubeconfig)
503503
kubeconfig.Wrap(s.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
504504
}
505+
for _, customOpenShiftRoundTripper := range s.OpenShiftContext.CustomRoundTrippers {
506+
kubeconfig.Wrap(customOpenShiftRoundTripper)
507+
}
505508

506509
client, err := clientset.NewForConfig(restclient.AddUserAgent(kubeconfig, KubeControllerManagerUserAgent))
507510
if err != nil {

cmd/kube-controller-manager/app/patch.go

+32-1
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@ package app
33
import (
44
"fmt"
55
"io/ioutil"
6+
"net/http"
67
"path"
8+
"strings"
79
"time"
810

911
"k8s.io/apimachinery/pkg/util/json"
1012
kyaml "k8s.io/apimachinery/pkg/util/yaml"
1113
"k8s.io/client-go/informers"
1214
"k8s.io/client-go/rest"
1315
"k8s.io/client-go/tools/clientcmd"
16+
"k8s.io/client-go/transport"
1417
"k8s.io/component-base/metrics/legacyregistry"
1518
"k8s.io/kubernetes/cmd/kube-controller-manager/app/config"
1619
"k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
@@ -21,7 +24,9 @@ import (
2124

2225
var InformerFactoryOverride informers.SharedInformerFactory
2326

24-
func SetUpPreferredHostForOpenShift(controllerManagerOptions *options.KubeControllerManagerOptions) error {
27+
func SetUpCustomRoundTrippersForOpenShift(controllerManagerOptions *options.KubeControllerManagerOptions) error {
28+
controllerManagerOptions.OpenShiftContext.CustomRoundTrippers = []transport.WrapperFunc{newRejectIfNotReadyHeaderRoundTripper([]string{"generic-garbage-collector", "namespace-controller"})}
29+
2530
if !controllerManagerOptions.OpenShiftContext.UnsupportedKubeAPIOverPreferredHost {
2631
return nil
2732
}
@@ -54,6 +59,7 @@ func SetUpPreferredHostForOpenShift(controllerManagerOptions *options.KubeContro
5459

5560
controllerManagerOptions.Authentication.WithCustomRoundTripper(controllerManagerOptions.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
5661
controllerManagerOptions.Authorization.WithCustomRoundTripper(controllerManagerOptions.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
62+
5763
return nil
5864
}
5965

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

134140
return &restConfigCopy
135141
}
142+
143+
// newRejectIfNotReadyHeaderRoundTripper a middleware for setting X-OpenShift-Internal-If-Not-Ready HTTP Header for the given users.
144+
// In general, setting the header will result in getting 429 when the server hasn't been ready.
145+
// This prevents certain controllers like GC, Namespace from accidentally removing resources when the caches haven't been fully synchronized.
146+
func newRejectIfNotReadyHeaderRoundTripper(eligibleUsers []string) func(http.RoundTripper) http.RoundTripper {
147+
return func(rt http.RoundTripper) http.RoundTripper {
148+
return &rejectIfNotReadyHeaderRT{baseRT: rt, eligibleUsers: eligibleUsers}
149+
}
150+
}
151+
152+
type rejectIfNotReadyHeaderRT struct {
153+
baseRT http.RoundTripper
154+
eligibleUsers []string
155+
}
156+
157+
func (rt *rejectIfNotReadyHeaderRT) RoundTrip(r *http.Request) (*http.Response, error) {
158+
currentUser := r.UserAgent()
159+
for _, eligibleUser := range rt.eligibleUsers {
160+
if strings.Contains(currentUser, eligibleUser) {
161+
r.Header.Set("X-OpenShift-Internal-If-Not-Ready", "reject")
162+
break
163+
}
164+
}
165+
return rt.baseRT.RoundTrip(r)
166+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package app
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/textproto"
7+
"testing"
8+
)
9+
10+
func TestRejectIfNotReadyHeaderRT(t *testing.T) {
11+
scenarios := []struct {
12+
name string
13+
eligibleUsers []string
14+
currentUser string
15+
expectHeader bool
16+
}{
17+
{
18+
name: "scenario 1: happy path",
19+
currentUser: "system:serviceaccount:kube-system:generic-garbage-collector",
20+
eligibleUsers: []string{"generic-garbage-collector", "namespace-controller"},
21+
expectHeader: true,
22+
},
23+
{
24+
name: "scenario 2: ineligible user",
25+
currentUser: "system:serviceaccount:kube-system:service-account-controller",
26+
eligibleUsers: []string{"generic-garbage-collector", "namespace-controller"},
27+
expectHeader: false,
28+
},
29+
}
30+
31+
for _, scenario := range scenarios {
32+
t.Run(scenario.name, func(t *testing.T) {
33+
// set up the test
34+
fakeRT := fakeRTFunc(func(r *http.Request) (*http.Response, error) {
35+
// this is where we validate if the header was set or not
36+
headerSet := func() bool {
37+
if len(r.Header.Get("X-OpenShift-Internal-If-Not-Ready")) > 0 {
38+
return true
39+
}
40+
return false
41+
}()
42+
if scenario.expectHeader && !headerSet {
43+
return nil, fmt.Errorf("%v header wasn't set", textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
44+
}
45+
if !scenario.expectHeader && headerSet {
46+
return nil, fmt.Errorf("didn't expect %v header", textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
47+
}
48+
if scenario.expectHeader {
49+
if value := r.Header.Get("X-OpenShift-Internal-If-Not-Ready"); value != "reject" {
50+
return nil, fmt.Errorf("unexpected value %v in the %v header, expected \"reject\"", value, textproto.CanonicalMIMEHeaderKey("X-OpenShift-Internal-If-Not-Ready"))
51+
}
52+
}
53+
return nil, nil
54+
})
55+
target := newRejectIfNotReadyHeaderRoundTripper(scenario.eligibleUsers)(fakeRT)
56+
req, err := http.NewRequest("GET", "", nil)
57+
if err != nil {
58+
t.Fatal(err)
59+
}
60+
req.Header.Set("User-Agent", scenario.currentUser)
61+
62+
// act and validate
63+
if _, err := target.RoundTrip(req); err != nil {
64+
t.Fatal(err)
65+
}
66+
})
67+
}
68+
}
69+
70+
type fakeRTFunc func(r *http.Request) (*http.Response, error)
71+
72+
func (rt fakeRTFunc) RoundTrip(r *http.Request) (*http.Response, error) {
73+
return rt(r)
74+
}

0 commit comments

Comments
 (0)