Skip to content

Commit 2ebf199

Browse files
p0lyn0mialsoltysh
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(v1.24):source=57afba23a2f openshift-rebase(v1.24):source=57afba23a2f openshift-rebase(v1.24):source=57afba23a2f
1 parent 3b2555a commit 2ebf199

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
@@ -132,7 +132,7 @@ controller, and serviceaccounts controller.`,
132132
}
133133
cliflag.PrintFlags(cmd.Flags())
134134

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

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

+3
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,9 @@ func (s KubeControllerManagerOptions) Config(allControllers []string, disabledBy
440440
libgorestclient.DefaultServerName(kubeconfig)
441441
kubeconfig.Wrap(s.OpenShiftContext.PreferredHostRoundTripperWrapperFn)
442442
}
443+
for _, customOpenShiftRoundTripper := range s.OpenShiftContext.CustomRoundTrippers {
444+
kubeconfig.Wrap(customOpenShiftRoundTripper)
445+
}
443446

444447
client, err := clientset.NewForConfig(restclient.AddUserAgent(kubeconfig, KubeControllerManagerUserAgent))
445448
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)