Skip to content

Commit ba6b055

Browse files
JefftreeJefftree
committed
Fixes
1 parent 39f24fc commit ba6b055

File tree

5 files changed

+85
-95
lines changed

5 files changed

+85
-95
lines changed

‎pkg/handler/handler.go‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ package handler
1919
import (
2020
"bytes"
2121
"compress/gzip"
22+
"crypto/sha512"
2223
"encoding/json"
24+
"fmt"
2325
"mime"
2426
"net/http"
2527
"strconv"
@@ -49,6 +51,13 @@ const (
4951
mimePbGz = "application/x-gzip"
5052
)
5153

54+
func computeETag(data []byte) string {
55+
if data == nil {
56+
return ""
57+
}
58+
return fmt.Sprintf("%X", sha512.Sum512(data))
59+
}
60+
5261
// OpenAPIService is the service responsible for serving OpenAPI spec. It has
5362
// the ability to safely change the spec while serving it.
5463
type OpenAPIService struct {
@@ -59,6 +68,7 @@ type OpenAPIService struct {
5968

6069
jsonCache handler.HandlerCache
6170
protoCache handler.HandlerCache
71+
etagCache handler.HandlerCache
6272
}
6373

6474
func init() {
@@ -79,43 +89,50 @@ func NewOpenAPIService(spec *spec.Swagger) (*OpenAPIService, error) {
7989
func (o *OpenAPIService) getSwaggerBytes() ([]byte, string, time.Time, error) {
8090
o.rwMutex.RLock()
8191
defer o.rwMutex.RUnlock()
82-
specBytes, etag, err := o.jsonCache.Get()
92+
specBytes, err := o.jsonCache.Get()
93+
if err != nil {
94+
return nil, "", time.Time{}, err
95+
}
96+
etagBytes, err := o.etagCache.Get()
8397
if err != nil {
8498
return nil, "", time.Time{}, err
8599
}
86-
return specBytes, etag, o.lastModified, nil
100+
return specBytes, string(etagBytes), o.lastModified, nil
87101
}
88102

89103
func (o *OpenAPIService) getSwaggerPbBytes() ([]byte, string, time.Time, error) {
90104
o.rwMutex.RLock()
91105
defer o.rwMutex.RUnlock()
92-
specPb, etag, err := o.protoCache.Get()
106+
specPb, err := o.protoCache.Get()
93107
if err != nil {
94108
return nil, "", time.Time{}, err
95109
}
96-
return specPb, etag, o.lastModified, nil
110+
etagBytes, err := o.etagCache.Get()
111+
if err != nil {
112+
return nil, "", time.Time{}, err
113+
}
114+
return specPb, string(etagBytes), o.lastModified, nil
97115
}
98116

99117
func (o *OpenAPIService) UpdateSpec(openapiSpec *spec.Swagger) (err error) {
100118
o.rwMutex.Lock()
101119
defer o.rwMutex.Unlock()
102-
o.jsonCache = o.jsonCache.New(func() ([]byte, string, error) {
103-
jsonData, err := json.Marshal(openapiSpec)
104-
if err != nil {
105-
return nil, "", err
106-
}
107-
return jsonData, "", nil
120+
o.jsonCache = o.jsonCache.New(func() ([]byte, error) {
121+
return json.Marshal(openapiSpec)
108122
})
109-
o.protoCache = o.protoCache.New(func() ([]byte, string, error) {
110-
json, etag, err := o.jsonCache.Get()
123+
o.protoCache = o.protoCache.New(func() ([]byte, error) {
124+
json, err := o.jsonCache.Get()
111125
if err != nil {
112-
return nil, "", err
126+
return nil, err
113127
}
114-
pb, err := ToProtoBinary(json)
128+
return ToProtoBinary(json)
129+
})
130+
o.etagCache = o.etagCache.New(func() ([]byte, error) {
131+
json, err := o.jsonCache.Get()
115132
if err != nil {
116-
return nil, "", err
133+
return nil, err
117134
}
118-
return pb, etag, nil
135+
return []byte(computeETag(json)), nil
119136
})
120137
o.lastModified = time.Now()
121138

‎pkg/handler3/handler.go‎

Lines changed: 42 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package handler3
1818

1919
import (
2020
"bytes"
21+
"crypto/sha512"
2122
"encoding/json"
2223
"fmt"
2324
"mime"
@@ -33,7 +34,6 @@ import (
3334
"github.com/golang/protobuf/proto"
3435
openapi_v3 "github.com/googleapis/gnostic/openapiv3"
3536
"github.com/munnerz/goautoneg"
36-
klog "k8s.io/klog/v2"
3737
"k8s.io/kube-openapi/pkg/common"
3838
"k8s.io/kube-openapi/pkg/internal/handler"
3939
"k8s.io/kube-openapi/pkg/spec3"
@@ -52,13 +52,6 @@ const (
5252
subTypeJSON = "json"
5353
)
5454

55-
func computeETag(data []byte) string {
56-
if data == nil {
57-
return ""
58-
}
59-
return fmt.Sprintf("%X", sha512.Sum512(data))
60-
}
61-
6255
// OpenAPIV3Discovery is the format of the Discovery document for OpenAPI V3
6356
// It maps Discovery paths to their corresponding URLs with a hash parameter included
6457
type OpenAPIV3Discovery struct {
@@ -87,6 +80,7 @@ type OpenAPIV3Group struct {
8780

8881
pbCache handler.HandlerCache
8982
jsonCache handler.HandlerCache
83+
etagCache handler.HandlerCache
9084
}
9185

9286
func init() {
@@ -95,22 +89,29 @@ func init() {
9589
mime.AddExtensionType(".gz", mimePbGz)
9690
}
9791

98-
// NewOpenAPIService builds an OpenAPIService starting with the given spec.
99-
func NewOpenAPIService(spec *spec.Swagger) (*OpenAPIService, error) {
100-
o := &OpenAPIService{}
101-
o.v3Schema = make(map[string]*OpenAPIV3Group)
102-
return o, nil
92+
func computeETag(data []byte) string {
93+
if data == nil {
94+
return ""
95+
}
96+
return fmt.Sprintf("%X", sha512.Sum512(data))
10397
}
10498

105-
func constructURL(r *http.Request, gvString, etag string) (string, error) {
99+
func constructURL(gvString, etag string) string {
106100
u := url.URL{Path: path.Join("/openapi/v3", gvString)}
107101
query := url.Values{}
108102
query.Set("hash", etag)
109103
u.RawQuery = query.Encode()
110-
return u.String(), nil
104+
return u.String()
111105
}
112106

113-
func (o *OpenAPIService) getGroupBytes(r *http.Request) ([]byte, error) {
107+
// NewOpenAPIService builds an OpenAPIService starting with the given spec.
108+
func NewOpenAPIService(spec *spec.Swagger) (*OpenAPIService, error) {
109+
o := &OpenAPIService{}
110+
o.v3Schema = make(map[string]*OpenAPIV3Group)
111+
return o, nil
112+
}
113+
114+
func (o *OpenAPIService) getGroupBytes() ([]byte, error) {
114115
o.rwMutex.RLock()
115116
defer o.rwMutex.RUnlock()
116117
keys := make([]string, len(o.v3Schema))
@@ -123,15 +124,13 @@ func (o *OpenAPIService) getGroupBytes(r *http.Request) ([]byte, error) {
123124
sort.Strings(keys)
124125
discovery := &OpenAPIV3Discovery{Paths: make(map[string]OpenAPIV3DiscoveryGroupVersion)}
125126
for gvString, groupVersion := range o.v3Schema {
126-
_, etag, err := groupVersion.jsonCache.Get()
127+
etagBytes, err := groupVersion.etagCache.Get()
127128
if err != nil {
128129
return nil, err
129130
}
130-
u, err := constructURL(r, gvString, etag)
131-
if err != nil {
132-
return nil, err
131+
discovery.Paths[gvString] = OpenAPIV3DiscoveryGroupVersion{
132+
URL: constructURL(gvString, string(etagBytes)),
133133
}
134-
discovery.Paths[gvString] = OpenAPIV3DiscoveryGroupVersion{URL: u}
135134
}
136135
j, err := json.Marshal(discovery)
137136
if err != nil {
@@ -148,15 +147,19 @@ func (o *OpenAPIService) getSingleGroupBytes(getType string, group string) ([]by
148147
return nil, "", time.Now(), fmt.Errorf("Cannot find CRD group %s", group)
149148
}
150149
if getType == subTypeJSON {
151-
specBytes, etag, err := v.jsonCache.Get()
152-
return specBytes, etag, v.lastModified, err
150+
specBytes, err := v.jsonCache.Get()
151+
if err != nil {
152+
return nil, "", v.lastModified, err
153+
}
154+
etagBytes, err := v.etagCache.Get()
155+
return specBytes, string(etagBytes), v.lastModified, err
153156
} else if getType == subTypeProtobuf {
154-
_, etag, err := v.jsonCache.Get()
157+
specPb, err := v.pbCache.Get()
155158
if err != nil {
156159
return nil, "", v.lastModified, err
157160
}
158-
specPb, _, err := v.pbCache.Get()
159-
return specPb, etag, v.lastModified, err
161+
etagBytes, err := v.etagCache.Get()
162+
return specPb, string(etagBytes), v.lastModified, err
160163
}
161164
return nil, "", time.Now(), fmt.Errorf("Invalid accept clause %s", getType)
162165
}
@@ -186,7 +189,7 @@ func ToV3ProtoBinary(json []byte) ([]byte, error) {
186189
}
187190

188191
func (o *OpenAPIService) HandleDiscovery(w http.ResponseWriter, r *http.Request) {
189-
data, _ := o.getGroupBytes(r)
192+
data, _ := o.getGroupBytes()
190193
http.ServeContent(w, r, "/openapi/v3", time.Now(), bytes.NewReader(data))
191194
}
192195

@@ -249,11 +252,7 @@ func (o *OpenAPIService) HandleGroupVersion(w http.ResponseWriter, r *http.Reque
249252
w.Header().Set("Etag", strconv.Quote(etag))
250253

251254
if CacheBustingShouldRedirect(r, etag) {
252-
u, err := constructURL(r, group, etag)
253-
if err != nil {
254-
klog.Errorf("Error parsing URL %s, %v", r.URL.String(), err)
255-
return
256-
}
255+
u := constructURL(group, etag)
257256
http.Redirect(w, r, u, 301)
258257
return
259258
}
@@ -276,24 +275,22 @@ func (o *OpenAPIV3Group) UpdateSpec(openapi *spec3.OpenAPI) (err error) {
276275
o.rwMutex.Lock()
277276
defer o.rwMutex.Unlock()
278277

279-
o.pbCache = o.pbCache.New(func() ([]byte, string, error) {
280-
json, etag, err := o.jsonCache.Get()
281-
if err != nil {
282-
return nil, "", err
283-
}
284-
pb, err := ToV3ProtoBinary(json)
278+
o.pbCache = o.pbCache.New(func() ([]byte, error) {
279+
json, err := o.jsonCache.Get()
285280
if err != nil {
286-
return nil, "", err
281+
return nil, err
287282
}
288-
return pb, etag, nil
283+
return ToV3ProtoBinary(json)
289284
})
290-
291-
o.jsonCache = o.jsonCache.New(func() ([]byte, string, error) {
292-
specBytes, err := json.Marshal(openapi)
285+
o.jsonCache = o.jsonCache.New(func() ([]byte, error) {
286+
return json.Marshal(openapi)
287+
})
288+
o.etagCache = o.etagCache.New(func() ([]byte, error) {
289+
json, err := o.jsonCache.Get()
293290
if err != nil {
294-
return nil, "", err
291+
return nil, err
295292
}
296-
return specBytes, "", nil
293+
return []byte(computeETag(json)), nil
297294
})
298295
o.lastModified = time.Now()
299296
return nil

‎pkg/handler3/handler_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ package handler3
1818

1919
import (
2020
"bytes"
21-
"crypto/sha512"
22-
"fmt"
2321
"io/ioutil"
2422
"net/http"
2523
"net/http/httptest"
@@ -40,13 +38,6 @@ var returnedOpenAPI = []byte(`{
4038
},
4139
"paths": {}}`)
4240

43-
func computeETag(data []byte) string {
44-
if data == nil {
45-
return ""
46-
}
47-
return fmt.Sprintf("%X", sha512.Sum512(data))
48-
}
49-
5041
func TestRegisterOpenAPIVersionedService(t *testing.T) {
5142
var s *spec3.OpenAPI
5243
buffer := new(bytes.Buffer)

‎pkg/internal/handler/handler_cache.go‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,22 @@ limitations under the License.
1717
package handler
1818

1919
import (
20-
"crypto/sha512"
21-
"fmt"
2220
"sync"
2321
)
2422

25-
func computeETag(data []byte) string {
26-
if data == nil {
27-
return ""
28-
}
29-
return fmt.Sprintf("\"%X\"", sha512.Sum512(data))
30-
}
31-
32-
// HandlerCache represents an OpenAPI v2/v3 marshaling cache.
23+
// HandlerCache represents a lazy cache for generating a byte array
24+
// It is used to lazily marshal OpenAPI v2/v3 and lazily generate the ETag
3325
type HandlerCache struct {
3426
BuildCache func() ([]byte, error)
3527
once sync.Once
3628
bytes []byte
37-
etag string
3829
err error
3930
}
4031

4132
// Get either returns the cached value or calls BuildCache() once before caching and returning
4233
// its results. If BuildCache returns an error, the last valid value for the cache (from prior
4334
// calls to New()) is used instead if possible.
44-
func (c *HandlerCache) Get() ([]byte, string, error) {
35+
func (c *HandlerCache) Get() ([]byte, error) {
4536
c.once.Do(func() {
4637
bytes, err := c.BuildCache()
4738
// if there is an error updating the cache, there can be situations where
@@ -51,18 +42,16 @@ func (c *HandlerCache) Get() ([]byte, string, error) {
5142
if c.err == nil {
5243
// don't override previous spec if we had an error
5344
c.bytes = bytes
54-
c.etag = computeETag(c.bytes)
5545
}
5646
})
57-
return c.bytes, c.etag, c.err
47+
return c.bytes, c.err
5848
}
5949

6050
// New creates a new HandlerCache for situations where a cache refresh is needed.
6151
// This function is not thread-safe and should not be called at the same time as Get().
6252
func (c *HandlerCache) New(cacheBuilder func() ([]byte, error)) HandlerCache {
6353
return HandlerCache{
6454
bytes: c.bytes,
65-
etag: c.etag,
6655
BuildCache: cacheBuilder,
6756
}
6857
}

0 commit comments

Comments
 (0)