Skip to content

Commit e1aee80

Browse files
Merge branch 'master' into port_spec2
2 parents 25b4930 + 99ae1fd commit e1aee80

30 files changed

Lines changed: 378 additions & 143 deletions

.circleci/config.yml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ integrationDefaults: &integrationDefaults
1111
CHANGE_MINIKUBE_NONE_USER: true
1212
HUB: docker.io/dnerepo
1313
TAG: dontpush
14-
GOPATH: /go
14+
GOPATH: /go
1515

1616
jobs:
1717
pilot-integration-auth:
@@ -22,7 +22,7 @@ jobs:
2222
pwd: /
2323
command: |
2424
sudo mkdir -p /go/src/istio.io/istio
25-
sudo chown -R circleci /go
25+
sudo chown -R circleci /go
2626
- checkout
2727
- run: curl -Lo kubectl https://storage.googleapis.com/kubernetes-release/release/v1.7.4/bin/linux/amd64/kubectl && chmod +x kubectl && sudo mv kubectl /usr/local/bin/
2828
- run: curl -Lo minikube https://storage.googleapis.com/minikube/releases/v0.22.3/minikube-linux-amd64 && chmod +x minikube && sudo mv minikube /usr/local/bin/
@@ -48,7 +48,7 @@ jobs:
4848
- run: JSONPATH='{range .items[*]}{@.metadata.name}:{range @.status.conditions[*]}{@.type}={@.status};{end}{end}'; until sudo kubectl get nodes -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; do sleep 1; done
4949
- run: sudo -E kubectl cluster-info
5050
- run: make setup
51-
- run: ./integration --logtostderr -hub $HUB -tag $TAG -mixer=false -auth=enable -errorlogsdir=/home/circleci/logs
51+
- run: ./integration --logtostderr -hub $HUB -tag $TAG -mixer=false -auth=enable -errorlogsdir=/home/circleci/logs -use-initializer
5252
- store_artifacts:
5353
path: /home/circleci/logs
5454

@@ -60,7 +60,7 @@ jobs:
6060
pwd: /
6161
command: |
6262
sudo mkdir -p /go/src/istio.io/istio
63-
sudo chown -R circleci /go
63+
sudo chown -R circleci /go
6464
- checkout
6565
- run: curl -Lo kubectl https://storage.googleapis.com/kubernetes-release/release/v1.7.4/bin/linux/amd64/kubectl && chmod +x kubectl && sudo mv kubectl /usr/local/bin/
6666
- run: curl -Lo minikube https://storage.googleapis.com/minikube/releases/v0.22.3/minikube-linux-amd64 && chmod +x minikube && sudo mv minikube /usr/local/bin/
@@ -86,7 +86,7 @@ jobs:
8686
- run: JSONPATH='{range .items[*]}{@.metadata.name}:{range @.status.conditions[*]}{@.type}={@.status};{end}{end}'; until sudo kubectl get nodes -o jsonpath="$JSONPATH" 2>&1 | grep -q "Ready=True"; do sleep 1; done
8787
- run: sudo -E kubectl cluster-info
8888
- run: make setup
89-
- run: ./integration --logtostderr -hub $HUB -tag $TAG -mixer=false -auth=disable -errorlogsdir=/home/circleci/logs
89+
- run: ./integration --logtostderr -hub $HUB -tag $TAG -mixer=false -auth=disable -errorlogsdir=/home/circleci/logs -use-initializer
9090
- store_artifacts:
9191
path: /home/circleci/logs
9292

@@ -110,7 +110,7 @@ jobs:
110110

111111
unittest-pilot:
112112
<<: *defaults
113-
resource_class: xlarge
113+
resource_class: xlarge
114114
steps:
115115
- checkout
116116
- run: mkdir -p /tmp/coverage
@@ -177,7 +177,7 @@ jobs:
177177
178178
unittest-security:
179179
<<: *defaults
180-
resource_class: xlarge
180+
resource_class: xlarge
181181
steps:
182182
- checkout
183183
- run: mkdir -p /tmp/coverage
@@ -206,7 +206,7 @@ jobs:
206206
207207
unittest-broker:
208208
<<: *defaults
209-
resource_class: xlarge
209+
resource_class: xlarge
210210
steps:
211211
- checkout
212212
- run: mkdir -p /tmp/coverage

WORKSPACE

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ go_repository(
10401040

10411041
# Change this and the pilot/docker/Dockerfile.proxy* files together
10421042
# This SHA is obtained from proxy/postsubmit job
1043-
ISTIO_PROXY_BUCKET = "ad3f963c6a197b8ad36c9f9428986c7fe84d20ca"
1043+
ISTIO_PROXY_BUCKET = "76ed00adea006e25878f20daa58c456848243999"
10441044

10451045
http_file(
10461046
name = "envoy_binary",

lintconfig_base.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
"exclude": [
2929
"vendor",
3030
"mixer/adapter/svcctrl",
31-
"^security",
3231
".pb.go",
3332
"mixer/pkg/config/proto/combined.go",
3433
".*.gen.go",

mixer/adapter/svcctrl/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ go_library(
2121
"//mixer/adapter/svcctrl/config:go_default_library",
2222
"//mixer/adapter/svcctrl/template/svcctrlreport:go_default_library",
2323
"//mixer/pkg/adapter:go_default_library",
24+
"//mixer/pkg/cache:go_default_library",
2425
"//mixer/pkg/status:go_default_library",
2526
"//mixer/template/apikey:go_default_library",
2627
"//mixer/template/quota:go_default_library",

mixer/adapter/svcctrl/checkprocessor.go

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727

2828
"istio.io/istio/mixer/adapter/svcctrl/config"
2929
"istio.io/istio/mixer/pkg/adapter"
30+
"istio.io/istio/mixer/pkg/cache"
3031
"istio.io/istio/mixer/pkg/status"
3132
"istio.io/istio/mixer/template/apikey"
3233
)
@@ -35,9 +36,19 @@ import (
3536
type checkImpl struct {
3637
env adapter.Env
3738
checkResultExpiration time.Duration
38-
runtimeConfig *config.RuntimeConfig
39-
serviceConfig *config.GcpServiceSetting
40-
client serviceControlClient
39+
// A LRU cache, keyed by checkCacheKey struct and value is of type *sc.CheckResponse
40+
responseCache cache.ExpiringCache
41+
runtimeConfig *config.RuntimeConfig
42+
serviceConfig *config.GcpServiceSetting
43+
client serviceControlClient
44+
}
45+
46+
// Cache key used with checkImpl.responseCache. The cache is at handler level, so it stores check responses for multiple
47+
// GCP services.
48+
type checkCacheKey struct {
49+
googleServiceName string
50+
consumerID string
51+
operation string
4152
}
4253

4354
// ProcessCheck processes check call and converts CheckResponse to adapter.CheckResult.
@@ -48,21 +59,10 @@ func (c *checkImpl) ProcessCheck(ctx context.Context, instance *apikey.Instance)
4859
fmt.Sprintf(
4960
"instance:%s, api key and api operation must not be empty", instance.Name))), nil
5061
}
51-
5262
consumerID := generateConsumerIDFromAPIKey(instance.ApiKey)
5363
response, err := c.doCheck(consumerID, instance.ApiOperation, instance.Timestamp)
5464
if err != nil {
55-
return c.checkResult(
56-
rpc.Status{
57-
Code: int32(rpc.PERMISSION_DENIED),
58-
Message: err.Error(),
59-
}), nil
60-
}
61-
62-
if c.env.Logger().VerbosityLevel(logDebug) {
63-
if responseDetail, err := toFormattedJSON(response); err == nil {
64-
c.env.Logger().Infof("response: %v", string(responseDetail))
65-
}
65+
return c.checkResult(status.WithPermissionDenied(err.Error())), nil
6666
}
6767

6868
return c.responseToCheckResult(response)
@@ -85,6 +85,17 @@ func (c *checkImpl) ResolveConsumerProjectID(consumerID, opName string) (string,
8585

8686
// doCheck calls Check on Google ServiceControl client.
8787
func (c *checkImpl) doCheck(consumerID, operationName string, timestamp time.Time) (*sc.CheckResponse, error) {
88+
cacheKey := checkCacheKey{
89+
googleServiceName: c.serviceConfig.GoogleServiceName,
90+
consumerID: consumerID,
91+
operation: operationName,
92+
}
93+
94+
cachedResponse, found := c.responseCache.Get(cacheKey)
95+
if found {
96+
return cachedResponse.(*sc.CheckResponse), nil
97+
}
98+
8899
request := &sc.CheckRequest{
89100
Operation: &sc.Operation{
90101
OperationId: uuid.New(),
@@ -93,7 +104,26 @@ func (c *checkImpl) doCheck(consumerID, operationName string, timestamp time.Tim
93104
ConsumerId: consumerID,
94105
},
95106
}
96-
return c.client.Check(c.serviceConfig.GoogleServiceName, request)
107+
108+
if c.env.Logger().VerbosityLevel(logDebug) {
109+
if requestDetail, err := toFormattedJSON(request); err == nil {
110+
c.env.Logger().Infof("request: %v", string(requestDetail))
111+
}
112+
}
113+
114+
response, err := c.client.Check(c.serviceConfig.GoogleServiceName, request)
115+
if err != nil {
116+
return nil, err
117+
}
118+
119+
if c.env.Logger().VerbosityLevel(logDebug) {
120+
if responseDetail, err := toFormattedJSON(response); err == nil {
121+
c.env.Logger().Infof("response: %v", string(responseDetail))
122+
}
123+
}
124+
125+
c.responseCache.Set(cacheKey, response)
126+
return response, nil
97127
}
98128

99129
// responseToCheckResult converts ServiceControl CheckResponse to Mixer CheckerResult
@@ -132,6 +162,7 @@ func newCheckProcessor(meshServiceName string, ctx *handlerContext) (*checkImpl,
132162
return &checkImpl{
133163
ctx.env,
134164
toDuration(ctx.config.RuntimeConfig.CheckResultExpiration),
165+
ctx.checkResponseCache,
135166
ctx.config.RuntimeConfig,
136167
serviceConfig,
137168
ctx.client,

mixer/adapter/svcctrl/checkprocessor_test.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ func TestProcessCheck(t *testing.T) {
9797
}
9898

9999
testProcessCheck(test, response, expectedResult, t)
100+
// Call again with nil response to test check cache.
101+
testProcessCheck(test, nil, expectedResult, t)
100102
}
101103

102104
func TestProcessCheckFailedHTTPCode(t *testing.T) {
@@ -146,7 +148,6 @@ func TestProcessCheckWithError(t *testing.T) {
146148

147149
func TestResolveConsumerProjectID(t *testing.T) {
148150
test := checkProcessorTestSetup(t)
149-
150151
test.mockClient.setCheckResponse(&sc.CheckResponse{
151152
ServerResponse: googleapi.ServerResponse{
152153
HTTPStatusCode: 200,
@@ -157,14 +158,27 @@ func TestResolveConsumerProjectID(t *testing.T) {
157158
},
158159
},
159160
})
160-
161-
id, err := test.checkProc.ResolveConsumerProjectID(apiKeyPrefix+"test_key", "/echo")
162-
if err != nil {
163-
t.Errorf("ResolveConsumerProjectID(...) failed with error %v", err)
161+
{
162+
id, err := test.checkProc.ResolveConsumerProjectID(apiKeyPrefix+"test_key", "/echo")
163+
if err != nil {
164+
t.Fatalf(`ResolveConsumerProjectID(...) failed with error %v`, err)
165+
}
166+
if id != fmt.Sprintf("project_number:%d", gcpConsumerProjectNumber) {
167+
t.Errorf(`unexpected consumer project ID:%v`, id)
168+
}
164169
}
165-
166-
if id != fmt.Sprintf("project_number:%d", gcpConsumerProjectNumber) {
167-
t.Errorf(`unexpected consumer project ID:%v`, id)
170+
{
171+
// Repeat the same test but set injected response to nil. Without check cache, the following
172+
// test would fail.
173+
test.mockClient.checkResponse = nil
174+
id, err := test.checkProc.ResolveConsumerProjectID(apiKeyPrefix+"test_key", "/echo")
175+
test.mockClient.setCheckResponse(nil)
176+
if err != nil {
177+
t.Fatalf(`ResolveConsumerProjectID(...) failed with error %v`, err)
178+
}
179+
if id != fmt.Sprintf("project_number:%d", gcpConsumerProjectNumber) {
180+
t.Errorf(`unexpected consumer project ID:%v`, id)
181+
}
168182
}
169183
}
170184

mixer/adapter/svcctrl/handler.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"istio.io/istio/mixer/adapter/svcctrl/config"
2727
"istio.io/istio/mixer/adapter/svcctrl/template/svcctrlreport"
2828
"istio.io/istio/mixer/pkg/adapter"
29+
"istio.io/istio/mixer/pkg/cache"
2930
"istio.io/istio/mixer/pkg/status"
3031
"istio.io/istio/mixer/template/apikey"
3132
"istio.io/istio/mixer/template/quota"
@@ -62,11 +63,10 @@ type (
6263
config *config.Params
6364
// A map keyed by mesh service name to service config in adapter config
6465
serviceConfigIndex map[string]*config.GcpServiceSetting
65-
66-
checkDataShape map[string]*apikey.Type
67-
reportDataShape map[string]*svcctrlreport.Type
68-
69-
client serviceControlClient
66+
checkDataShape map[string]*apikey.Type
67+
reportDataShape map[string]*svcctrlreport.Type
68+
checkResponseCache cache.ExpiringCache // A LRU cache for check response
69+
client serviceControlClient
7070
}
7171

7272
handler struct {

mixer/adapter/svcctrl/svcctrl.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ import (
2020
"fmt"
2121

2222
pbtypes "github.com/gogo/protobuf/types"
23-
multierror "github.com/hashicorp/go-multierror"
23+
"github.com/hashicorp/go-multierror"
2424

2525
"istio.io/istio/mixer/adapter/svcctrl/config"
2626
"istio.io/istio/mixer/adapter/svcctrl/template/svcctrlreport"
2727
"istio.io/istio/mixer/pkg/adapter"
28+
"istio.io/istio/mixer/pkg/cache"
2829
"istio.io/istio/mixer/template/apikey"
2930
"istio.io/istio/mixer/template/quota"
3031
)
@@ -160,10 +161,16 @@ func initializeHandlerContext(env adapter.Env, adapterCfg *config.Params,
160161
configIndex[cfg.MeshServiceName] = cfg
161162
}
162163

164+
cacheExp := toDuration(adapterCfg.RuntimeConfig.CheckResultExpiration)
165+
// Set eviction interval to half of expiration time. That said the cache would scan and evict expired entries every
166+
// half of expiation time period.
167+
checkCache := cache.NewLRU(cacheExp, cacheExp/2, int(adapterCfg.RuntimeConfig.CheckCacheSize))
168+
163169
return &handlerContext{
164170
env: env,
165171
config: adapterCfg,
166172
serviceConfigIndex: configIndex,
173+
checkResponseCache: checkCache,
167174
client: client,
168175
}, nil
169176
}

mixer/adapter/svcctrl/svcctrl_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ func TestInitializeHandlerContext(t *testing.T) {
4242
t.Errorf("expect serviceConfigIndex :%v, but get %v",
4343
expectedIdx, ctx.serviceConfigIndex)
4444
}
45+
if ctx.checkResponseCache == nil {
46+
t.Errorf("fail to initialize check cache")
47+
}
4548
}
4649

4750
func TestConfigValidation(t *testing.T) {

pilot/docker/Dockerfile.proxy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Change this and ../../WORKSPACE at the same time
2-
FROM gcr.io/istio-testing/envoy:ad3f963c6a197b8ad36c9f9428986c7fe84d20ca
2+
FROM gcr.io/istio-testing/envoy:76ed00adea006e25878f20daa58c456848243999
33
ADD pilot-agent /usr/local/bin/pilot-agent
44

55
COPY envoy_pilot.json /etc/istio/proxy/envoy_pilot.json

0 commit comments

Comments
 (0)