Skip to content

Commit bf15488

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 dd4ac52 commit bf15488

File tree

5 files changed

+111
-2
lines changed

5 files changed

+111
-2
lines changed

Diff for: 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
}

Diff for: cmd/kube-controller-manager/app/controllermanager.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ controller, and serviceaccounts controller.`,
137137
}
138138
cliflag.PrintFlags(cmd.Flags())
139139

140-
if err := SetUpPreferredHostForOpenShift(s); err != nil {
140+
if err := SetUpCustomRoundTrippersForOpenShift(s); err != nil {
141141
fmt.Fprintf(os.Stderr, "%v\n", err)
142142
os.Exit(1)
143143
}

Diff for: cmd/kube-controller-manager/app/options/options.go

+3
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,9 @@ func (s KubeControllerManagerOptions) Config(allControllers []string, disabledBy
508508
libgorestclient.DefaultServerName(kubeconfig)
509509
kubeconfig.Wrap(s.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
510510
}
511+
for _, customOpenShiftRoundTripper := range s.OpenShiftContext.CustomRoundTrippers {
512+
kubeconfig.Wrap(customOpenShiftRoundTripper)
513+
}
511514

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

Diff for: 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+
}

Diff for: cmd/kube-controller-manager/app/patch_test.go

+74
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)