Skip to content

Commit

Permalink
Revert "Cache TTL fixes"
Browse files Browse the repository at this point in the history 
This reverts commit 1219806.
  • Loading branch information
@ScottGarman
ScottGarman committed Aug 16, 2023
1 parent 7f22c7e commit 35a5121
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 100 deletions.
13 changes: 0 additions & 13 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vb
github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logfmt/logfmt v0.5.1/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ=
github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
Expand All @@ -197,8 +196,6 @@ github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJn
github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY=
github.com/go-playground/validator/v10 v10.2.0/go.mod h1:uOYAAleCW8F/7oMFd6aG0GOhaH6EGOAJShg8Id5JGkI=
github.com/go-playground/validator/v10 v10.10.0/go.mod h1:74x4gJWsvQexRdW8Pn3dXSGrTK4nAUsbPlLADvpJkos=
github.com/go-playground/validator/v10 v10.14.1 h1:9c50NUPC30zyuKprjL3vNZ0m5oG+jU0zvx4AqHGnv4k=
github.com/go-playground/validator/v10 v10.14.1/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU=
github.com/go-playground/validator/v10 v10.15.0 h1:nDU5XeOKtB3GEa+uB7GNYwhVKsgjAR7VgKoNB6ryXfw=
github.com/go-playground/validator/v10 v10.15.0/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU=
github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
Expand Down Expand Up @@ -737,8 +734,6 @@ golang.org/x/crypto v0.0.0-20220511200225-c6db032c6c88/go.mod h1:IxCIyHEi3zRg3s0
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220826181053-bd7e27e6170d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58=
golang.org/x/crypto v0.11.0 h1:6Ewdq3tDic1mg5xRO4milcWCfMVQhI4NkqWWvqejpuA=
golang.org/x/crypto v0.11.0/go.mod h1:xgJhtzW8F9jGdVFWZESrid1U1bjeNy4zgy5cRr/CIio=
golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk=
golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -829,8 +824,6 @@ golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4/go.mod h1:CfG3xpIq0wQ8r1q4Su
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
golang.org/x/net v0.12.0 h1:cfawfvKITfUsFCeJIHJrbSxpeu/E81khclypR0GVT50=
golang.org/x/net v0.12.0/go.mod h1:zEVYFnQC7m/vmpQFELhcD1EWkZlX69l4oqgmer6hfKA=
golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14=
golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
Expand All @@ -853,8 +846,6 @@ golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ
golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.10.0 h1:zHCpF2Khkwy4mMB4bv0U37YtJdTGW8jI0glAApi0Kh8=
golang.org/x/oauth2 v0.10.0/go.mod h1:kTpgurOux7LqtuxjuyZa4Gj2gdezIt/jQtGnNFfypQI=
golang.org/x/oauth2 v0.11.0 h1:vPL4xzxBM4niKCW6g9whtaWVXTJf1U5e4aZxxFx/gbU=
golang.org/x/oauth2 v0.11.0/go.mod h1:LdF7O/8bLR/qWK9DrpXmbHLTouvRHK0SgJl0GmDBchk=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down Expand Up @@ -960,8 +951,6 @@ golang.org/x/sys v0.0.0-20220825204002-c680a09ffe64/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
Expand All @@ -978,8 +967,6 @@ golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/text v0.11.0 h1:LAntKIrcmeSKERyiOh0XMV39LXS8IE9UL2yP7+f5ij4=
golang.org/x/text v0.11.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/text v0.12.0 h1:k+n5B8goJNdU7hSvEtMUz3d1Q6D/XW4COJSJR6fN0mc=
golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
Expand Down
2 changes: 1 addition & 1 deletion internal/middleware/identify_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func IdentifyInstanceByIP(logger *zap.Logger, db *sqlx.DB) gin.HandlerFunc {
}

if instanceIPAddress != nil {
// We found the row, set the instance ID into the gin context.
// We found the row, set the instnace ID into the gin context.
c.Set(ContextKeyInstanceID, instanceIPAddress.InstanceID)
}
}
Expand Down
34 changes: 2 additions & 32 deletions pkg/api/v1/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ import (
"reflect"
"strings"
"text/template"
"time"

"github.com/gin-gonic/gin"
"github.com/go-playground/validator/v10"
"github.com/google/uuid"
"github.com/jmoiron/sqlx"
"github.com/spf13/viper"
"go.uber.org/zap"

"go.hollow.sh/toolbox/ginjwt"
Expand Down Expand Up @@ -122,25 +120,11 @@ func (r *Router) getMetadata(c *gin.Context) (*models.InstanceMetadatum, error)
return nil, errNotFound
}

metadataNotFound := false

// We got an instance ID from the middleware, either because we could match
// the request IP to an ID, or the request itself provided the instance ID.
metadata, err := models.FindInstanceMetadatum(c.Request.Context(), r.DB, instanceID)
if err != nil && errors.Is(err, sql.ErrNoRows) {
metadataNotFound = true
}

// Treat metadata that's older than our TTL as non-existent, to ensure we
// retrieve it using the lookup service
cacheTTL := viper.GetDuration("cache_ttl")
if err == nil && metadata != nil && cacheTTL != 0 && metadata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("TTL exceeded for instance ", metadata.ID, " not returning cached metadata")

metadataNotFound = true
}

if metadataNotFound {
if err != nil && errors.Is(err, sql.ErrNoRows) {
// We couldn't find an instance_metadata row for this instance ID. Try
// to fetch it from the upstream lookup service (if enabled and configured)
middleware.MetricMetadataCacheMiss.Inc()
Expand Down Expand Up @@ -184,25 +168,11 @@ func (r *Router) getUserdata(c *gin.Context) (*models.InstanceUserdatum, error)
return nil, errNotFound
}

userdataNotFound := false

// We got an instance ID from the middleware, either because we could match
// the request IP to an ID, or the request itself provided the instance ID.
userdata, err := models.FindInstanceUserdatum(c.Request.Context(), r.DB, instanceID)
if err != nil && errors.Is(err, sql.ErrNoRows) {
userdataNotFound = true
}

// Treat userdata that's older than our TTL as non-existent, to ensure we
// retrieve it using the lookup service
cacheTTL := viper.GetDuration("cache_ttl")
if err == nil && userdata != nil && cacheTTL != 0 && userdata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("TTL exceeded for instance ", userdata.ID, " not returning cached userdata")

userdataNotFound = true
}

if userdataNotFound {
if err != nil && errors.Is(err, sql.ErrNoRows) {
// We couldn't find an instance_metadata row for this instance ID. Try
// to fetch it from the upstream lookup service (if enabled and configured)
if r.LookupEnabled && r.LookupClient != nil {
Expand Down
26 changes: 22 additions & 4 deletions pkg/api/v1/router_instance_metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ func (r *Router) instanceMetadataGet(c *gin.Context) {
return
}

// Treat metadata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if metadata != nil && cacheTTL != 0 && metadata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Metadata for ", metadata.ID, " is older than our TTL, ignoring")
notFoundResponse(c)

return
}

if metadata != nil {
augmentedMetadata, err := addTemplateFields(metadata.Metadata, r.TemplateFields)
if err != nil {
Expand Down Expand Up @@ -140,10 +149,10 @@ func (r *Router) instanceMetadataExistsInternal(c *gin.Context) {
return
}

// Treat metadata that's older than our TTL as non-existent
// Treat metadata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if metadata != nil && cacheTTL != 0 && metadata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("TTL exceeded for instance ", metadata.ID, " not returning cached metadata (internal exists check)")
r.Logger.Sugar().Info("Metadata for ", metadata.ID, " is older than our TTL, ignoring (exists check)")
c.Status(http.StatusNotFound)

return
Expand Down Expand Up @@ -174,6 +183,15 @@ func (r *Router) instanceUserdataGet(c *gin.Context) {
return
}

// Treat userdata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if userdata != nil && cacheTTL != 0 && userdata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Userdata for ", userdata.ID, " is older than our TTL, ignoring")
notFoundResponse(c)

return
}

if userdata != nil {
c.String(http.StatusOK, string(userdata.Userdata.Bytes))
} else {
Expand Down Expand Up @@ -228,10 +246,10 @@ func (r *Router) instanceUserdataExistsInternal(c *gin.Context) {
return
}

// Treat userdata that's older than our TTL as non-existent
// Treat userdata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if userdata != nil && cacheTTL != 0 && userdata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("TTL exceeded for instance ", userdata.ID, " not returning cached userdata (internal exists check)")
r.Logger.Sugar().Info("Userdata for ", userdata.ID, " is older than our TTL, ignoring (exists check)")
c.Status(http.StatusNotFound)

return
Expand Down
38 changes: 38 additions & 0 deletions pkg/api/v1/router_instance_metadata_lookup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/lookup"
Expand Down Expand Up @@ -70,4 +72,40 @@ func TestGetMetadataLookupByIP(t *testing.T) {
assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetMetadataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

if testcase.expectedStatus != http.StatusInternalServerError {
testcase.expectedStatus = http.StatusNotFound
}

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Setting cache_ttl to zero should disable the TTL behavior
viper.Set("cache_ttl", 0)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetMetadataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}
}
25 changes: 0 additions & 25 deletions pkg/api/v1/router_instance_metadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import (
"regexp"
"testing"
"text/template"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/dbtools"
Expand Down Expand Up @@ -698,29 +696,6 @@ func TestGetMetadataInternal(t *testing.T) {
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

// Now our HEAD requests should behave as if cached metadata no longer exists
for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodHead, v1api.GetInternalMetadataByIDPath(testcase.instanceID), nil)
router.ServeHTTP(w, req)

expectedStatus := testcase.expectedStatus
if expectedStatus == http.StatusOK {
// Flip the expected 200 responses to 404s
expectedStatus = http.StatusNotFound
}
assert.Equal(t, expectedStatus, w.Code)
})
}

// Disable cache for remaining tests
viper.Set("cache_ttl", 0)

// GET request tests
for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
Expand Down
38 changes: 38 additions & 0 deletions pkg/api/v1/router_instance_userdata_lookup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/lookup"
Expand Down Expand Up @@ -70,4 +72,40 @@ func TestGetUserdataLookupByIP(t *testing.T) {
assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetUserdataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

if testcase.expectedStatus == http.StatusOK {
testcase.expectedStatus = http.StatusNotFound
}

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Setting cache_ttl to zero should disable the TTL behavior
viper.Set("cache_ttl", 0)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetUserdataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}
}
25 changes: 0 additions & 25 deletions pkg/api/v1/router_instance_userdata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import (
"net/http/httptest"
"regexp"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/dbtools"
Expand Down Expand Up @@ -478,29 +476,6 @@ func TestGetUserdataInternal(t *testing.T) {
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

// Now our HEAD requests should behave as if cached userdata no longer exists
for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodHead, v1api.GetInternalUserdataByIDPath(testcase.instanceID), nil)
router.ServeHTTP(w, req)

expectedStatus := testcase.expectedStatus
if expectedStatus == http.StatusOK {
// Flip the expected 200 responses to 404s
expectedStatus = http.StatusNotFound
}
assert.Equal(t, expectedStatus, w.Code)
})
}

// Disable cache for remaining tests
viper.Set("cache_ttl", 0)

// GET request tests
for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
Expand Down

0 comments on commit 35a5121

Please sign in to comment.