Skip to content

Commit ea3be4f

Browse files
ChihweiLHBirdbeeme1mrtoddbaert
authored
feat: Add support for HTTP eTag header and 304 no change response (#1645)
## This PR - adds the support for the `eTag` request header and `304 Not Modified` response. ### Related Issues Fixes #1558 ### Notes This proposal includes some significant behavior changes; therefore, any feedback, opinions, or objections are welcome and appreciated. ### How to test ```bash make build cd bin && ./flagd start --port 8013 --uri https://raw.githubusercontent.com/open-feature/flagd/main/samples/example_flags.flagd.json --debug ``` More specific test cases to be added when we all agree on proceeding with the implementation of the change in this PR. --------- Signed-off-by: Zhiwei Liang <[email protected]> Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Michael Beemer <[email protected]> Co-authored-by: Todd Baert <[email protected]>
1 parent ac647e0 commit ea3be4f

File tree

3 files changed

+131
-36
lines changed

3 files changed

+131
-36
lines changed

‎core/pkg/sync/http/http_sync.go‎

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type Sync struct {
2727
AuthHeader string
2828
Interval uint32
2929
ready bool
30+
eTag string
3031
}
3132

3233
// Client defines the behaviour required of a http client
@@ -42,7 +43,7 @@ type Cron interface {
4243
}
4344

4445
func (hs *Sync) ReSync(ctx context.Context, dataSync chan<- sync.DataSync) error {
45-
msg, err := hs.Fetch(ctx)
46+
msg, _, err := hs.fetchBody(ctx, true)
4647
if err != nil {
4748
return err
4849
}
@@ -63,7 +64,7 @@ func (hs *Sync) IsReady() bool {
6364

6465
func (hs *Sync) Sync(ctx context.Context, dataSync chan<- sync.DataSync) error {
6566
// Initial fetch
66-
fetch, err := hs.Fetch(ctx)
67+
fetch, _, err := hs.fetchBody(ctx, true)
6768
if err != nil {
6869
return err
6970
}
@@ -74,28 +75,25 @@ func (hs *Sync) Sync(ctx context.Context, dataSync chan<- sync.DataSync) error {
7475
hs.Logger.Debug(fmt.Sprintf("polling %s every %d seconds", hs.URI, hs.Interval))
7576
_ = hs.Cron.AddFunc(fmt.Sprintf("*/%d * * * *", hs.Interval), func() {
7677
hs.Logger.Debug(fmt.Sprintf("fetching configuration from %s", hs.URI))
77-
body, err := hs.fetchBodyFromURL(ctx, hs.URI)
78+
previousBodySHA := hs.LastBodySHA
79+
body, noChange, err := hs.fetchBody(ctx, false)
7880
if err != nil {
79-
hs.Logger.Error(err.Error())
81+
hs.Logger.Error(fmt.Sprintf("error fetching: %s", err.Error()))
8082
return
8183
}
8284

83-
if body == "" {
85+
if body == "" && !noChange {
8486
hs.Logger.Debug("configuration deleted")
8587
return
8688
}
8789

88-
currentSHA := hs.generateSha([]byte(body))
89-
90-
if hs.LastBodySHA == "" {
91-
hs.Logger.Debug("new configuration created")
90+
if previousBodySHA == "" {
91+
hs.Logger.Debug("configuration created")
9292
dataSync <- sync.DataSync{FlagData: body, Source: hs.URI}
93-
} else if hs.LastBodySHA != currentSHA {
94-
hs.Logger.Debug("configuration modified")
93+
} else if previousBodySHA != hs.LastBodySHA {
94+
hs.Logger.Debug("configuration updated")
9595
dataSync <- sync.DataSync{FlagData: body, Source: hs.URI}
9696
}
97-
98-
hs.LastBodySHA = currentSHA
9997
})
10098

10199
hs.Cron.Start()
@@ -108,10 +106,14 @@ func (hs *Sync) Sync(ctx context.Context, dataSync chan<- sync.DataSync) error {
108106
return nil
109107
}
110108

111-
func (hs *Sync) fetchBodyFromURL(ctx context.Context, url string) (string, error) {
112-
req, err := http.NewRequestWithContext(ctx, "GET", url, bytes.NewBuffer(nil))
109+
func (hs *Sync) fetchBody(ctx context.Context, fetchAll bool) (string, bool, error) {
110+
if hs.URI == "" {
111+
return "", false, errors.New("no HTTP URL string set")
112+
}
113+
114+
req, err := http.NewRequestWithContext(ctx, "GET", hs.URI, bytes.NewBuffer(nil))
113115
if err != nil {
114-
return "", fmt.Errorf("error creating request for url %s: %w", url, err)
116+
return "", false, fmt.Errorf("error creating request for url %s: %w", hs.URI, err)
115117
}
116118

117119
req.Header.Add("Accept", "application/json")
@@ -124,32 +126,50 @@ func (hs *Sync) fetchBodyFromURL(ctx context.Context, url string) (string, error
124126
req.Header.Set("Authorization", bearer)
125127
}
126128

129+
if hs.eTag != "" && !fetchAll {
130+
req.Header.Set("If-None-Match", hs.eTag)
131+
}
132+
127133
resp, err := hs.Client.Do(req)
128134
if err != nil {
129-
return "", fmt.Errorf("error calling endpoint %s: %w", url, err)
135+
return "", false, fmt.Errorf("error calling endpoint %s: %w", hs.URI, err)
130136
}
131137
defer func() {
132138
err = resp.Body.Close()
133139
if err != nil {
134-
hs.Logger.Debug(fmt.Sprintf("error closing the response body: %s", err.Error()))
140+
hs.Logger.Error(fmt.Sprintf("error closing the response body: %s", err.Error()))
135141
}
136142
}()
137143

144+
if resp.StatusCode == 304 {
145+
hs.Logger.Debug("no changes detected")
146+
return "", true, nil
147+
}
148+
138149
statusOK := resp.StatusCode >= 200 && resp.StatusCode < 300
139150
if !statusOK {
140-
return "", fmt.Errorf("error fetching from url %s: %s", url, resp.Status)
151+
return "", false, fmt.Errorf("error fetching from url %s: %s", hs.URI, resp.Status)
152+
}
153+
154+
if resp.Header.Get("ETag") != "" {
155+
hs.eTag = resp.Header.Get("ETag")
141156
}
142157

143158
body, err := io.ReadAll(resp.Body)
144159
if err != nil {
145-
return "", fmt.Errorf("unable to read body to bytes: %w", err)
160+
return "", false, fmt.Errorf("unable to read body to bytes: %w", err)
146161
}
147162

148-
json, err := utils.ConvertToJSON(body, getFileExtensions(url), resp.Header.Get("Content-Type"))
163+
json, err := utils.ConvertToJSON(body, getFileExtensions(hs.URI), resp.Header.Get("Content-Type"))
149164
if err != nil {
150-
return "", fmt.Errorf("error converting response body to json: %w", err)
165+
return "", false, fmt.Errorf("error converting response body to json: %w", err)
151166
}
152-
return json, nil
167+
168+
if json != "" {
169+
hs.LastBodySHA = hs.generateSha([]byte(body))
170+
}
171+
172+
return json, false, nil
153173
}
154174

155175
// getFileExtensions returns the file extension from the URL path
@@ -169,17 +189,6 @@ func (hs *Sync) generateSha(body []byte) string {
169189
}
170190

171191
func (hs *Sync) Fetch(ctx context.Context) (string, error) {
172-
if hs.URI == "" {
173-
return "", errors.New("no HTTP URL string set")
174-
}
175-
176-
body, err := hs.fetchBodyFromURL(ctx, hs.URI)
177-
if err != nil {
178-
return "", err
179-
}
180-
if body != "" {
181-
hs.LastBodySHA = hs.generateSha([]byte(body))
182-
}
183-
184-
return body, nil
192+
body, _, err := hs.fetchBody(ctx, false)
193+
return body, err
185194
}

‎core/pkg/sync/http/http_sync_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ func TestHTTPSync_Fetch(t *testing.T) {
110110
uri string
111111
bearerToken string
112112
authHeader string
113+
eTagHeader string
113114
lastBodySHA string
114115
handleResponse func(*testing.T, Sync, string, error)
115116
}{
@@ -240,6 +241,85 @@ func TestHTTPSync_Fetch(t *testing.T) {
240241
}
241242
},
242243
},
244+
"not modified response etag matched": {
245+
setup: func(t *testing.T, client *syncmock.MockClient) {
246+
expectedIfNoneMatch := `"1af17a664e3fa8e419b8ba05c2a173169df76162a5a286e0c405b460d478f7ef"`
247+
client.EXPECT().Do(gomock.Any()).DoAndReturn(func(req *http.Request) (*http.Response, error) {
248+
actualIfNoneMatch := req.Header.Get("If-None-Match")
249+
if actualIfNoneMatch != expectedIfNoneMatch {
250+
t.Fatalf("expected If-None-Match header to be '%s', got %s", expectedIfNoneMatch, actualIfNoneMatch)
251+
}
252+
return &http.Response{
253+
Header: map[string][]string{"ETag": {expectedIfNoneMatch}},
254+
Body: io.NopCloser(strings.NewReader("")),
255+
StatusCode: http.StatusNotModified,
256+
}, nil
257+
})
258+
},
259+
uri: "http://localhost",
260+
eTagHeader: `"1af17a664e3fa8e419b8ba05c2a173169df76162a5a286e0c405b460d478f7ef"`,
261+
handleResponse: func(t *testing.T, httpSync Sync, _ string, err error) {
262+
if err != nil {
263+
t.Fatalf("fetch: %v", err)
264+
}
265+
266+
expectedLastBodySHA := ""
267+
expectedETag := `"1af17a664e3fa8e419b8ba05c2a173169df76162a5a286e0c405b460d478f7ef"`
268+
if httpSync.LastBodySHA != expectedLastBodySHA {
269+
t.Errorf(
270+
"expected last body sha to be: '%s', got: '%s'", expectedLastBodySHA, httpSync.LastBodySHA,
271+
)
272+
}
273+
if httpSync.eTag != expectedETag {
274+
t.Errorf(
275+
"expected last etag to be: '%s', got: '%s'", expectedETag, httpSync.eTag,
276+
)
277+
}
278+
},
279+
},
280+
"modified response etag mismatched": {
281+
setup: func(t *testing.T, client *syncmock.MockClient) {
282+
expectedIfNoneMatch := `"1af17a664e3fa8e419b8ba05c2a173169df76162a5a286e0c405b460d478f7ef"`
283+
client.EXPECT().Do(gomock.Any()).DoAndReturn(func(req *http.Request) (*http.Response, error) {
284+
actualIfNoneMatch := req.Header.Get("If-None-Match")
285+
if actualIfNoneMatch != expectedIfNoneMatch {
286+
t.Fatalf("expected If-None-Match header to be '%s', got %s", expectedIfNoneMatch, actualIfNoneMatch)
287+
}
288+
289+
newContent := "\"Hey there!\""
290+
newETag := `"c2e01ce63d90109c4c7f4f6dcea97ed1bb2b51e3647f36caf5acbe27413a24bb"`
291+
292+
return &http.Response{
293+
Header: map[string][]string{
294+
"Content-Type": {"application/json"},
295+
"Etag": {newETag},
296+
},
297+
Body: io.NopCloser(strings.NewReader(newContent)),
298+
StatusCode: http.StatusOK,
299+
}, nil
300+
})
301+
},
302+
uri: "http://localhost",
303+
eTagHeader: `"1af17a664e3fa8e419b8ba05c2a173169df76162a5a286e0c405b460d478f7ef"`,
304+
handleResponse: func(t *testing.T, httpSync Sync, _ string, err error) {
305+
if err != nil {
306+
t.Fatalf("fetch: %v", err)
307+
}
308+
309+
expectedLastBodySHA := "wuAc5j2QEJxMf09tzql-0bsrUeNkfzbK9ay-J0E6JLs="
310+
expectedETag := `"c2e01ce63d90109c4c7f4f6dcea97ed1bb2b51e3647f36caf5acbe27413a24bb"`
311+
if httpSync.LastBodySHA != expectedLastBodySHA {
312+
t.Errorf(
313+
"expected last body sha to be: '%s', got: '%s'", expectedLastBodySHA, httpSync.LastBodySHA,
314+
)
315+
}
316+
if httpSync.eTag != expectedETag {
317+
t.Errorf(
318+
"expected last etag to be: '%s', got: '%s'", expectedETag, httpSync.eTag,
319+
)
320+
}
321+
},
322+
},
243323
}
244324

245325
for name, tt := range tests {
@@ -255,6 +335,7 @@ func TestHTTPSync_Fetch(t *testing.T) {
255335
AuthHeader: tt.authHeader,
256336
LastBodySHA: tt.lastBodySHA,
257337
Logger: logger.NewLogger(nil, false),
338+
eTag: tt.eTagHeader,
258339
}
259340

260341
fetched, err := httpSync.Fetch(context.Background())

‎docs/concepts/syncs.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ invoked with **HTTP GET** request.
3434
The polling interval, port, TLS settings, and authentication information can be configured.
3535
See [sync source](../reference/sync-configuration.md#source-configuration) configuration for details.
3636

37+
To optimize network usage, it honors the HTTP ETag protocol: if the server includes an `ETag` header in its response,
38+
flagd will store this value and send it in the `If-None-Match` header on subsequent requests. If the flag data has
39+
not changed, the server responds with 304 Not Modified, and flagd will skip updating its state. If the data has
40+
changed, the server returns the new content and a new ETag, prompting flagd to update its flags.
41+
3742
---
3843

3944
### gRPC sync

0 commit comments

Comments
 (0)