diff --git a/go.sum b/go.sum index a2bec2c..b070790 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= @@ -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= @@ -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= @@ -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= @@ -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= @@ -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= diff --git a/internal/middleware/identify_instance.go b/internal/middleware/identify_instance.go index b64cfd8..2c48535 100644 --- a/internal/middleware/identify_instance.go +++ b/internal/middleware/identify_instance.go @@ -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) } } diff --git a/pkg/api/v1/router.go b/pkg/api/v1/router.go index 59f8890..42006d3 100644 --- a/pkg/api/v1/router.go +++ b/pkg/api/v1/router.go @@ -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" @@ -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() @@ -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 { diff --git a/pkg/api/v1/router_instance_metadata.go b/pkg/api/v1/router_instance_metadata.go index 4aa2bac..afd2f6a 100644 --- a/pkg/api/v1/router_instance_metadata.go +++ b/pkg/api/v1/router_instance_metadata.go @@ -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 { @@ -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 @@ -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 { @@ -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 diff --git a/pkg/api/v1/router_instance_metadata_lookup_test.go b/pkg/api/v1/router_instance_metadata_lookup_test.go index 2ee46f0..3ccba0b 100644 --- a/pkg/api/v1/router_instance_metadata_lookup_test.go +++ b/pkg/api/v1/router_instance_metadata_lookup_test.go @@ -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" @@ -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) + }) + } } diff --git a/pkg/api/v1/router_instance_metadata_test.go b/pkg/api/v1/router_instance_metadata_test.go index 57eb7f9..03b5c83 100644 --- a/pkg/api/v1/router_instance_metadata_test.go +++ b/pkg/api/v1/router_instance_metadata_test.go @@ -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" @@ -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) { diff --git a/pkg/api/v1/router_instance_userdata_lookup_test.go b/pkg/api/v1/router_instance_userdata_lookup_test.go index cb4ecd7..5a4fde8 100644 --- a/pkg/api/v1/router_instance_userdata_lookup_test.go +++ b/pkg/api/v1/router_instance_userdata_lookup_test.go @@ -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" @@ -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) + }) + } } diff --git a/pkg/api/v1/router_instance_userdata_test.go b/pkg/api/v1/router_instance_userdata_test.go index dcc1b7d..d6e8c42 100644 --- a/pkg/api/v1/router_instance_userdata_test.go +++ b/pkg/api/v1/router_instance_userdata_test.go @@ -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" @@ -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) {