Skip to content

Commit

Permalink
Update some shas and use gogo options. (istio#503)
Browse files Browse the repository at this point in the history
* Update some shas.

- Get latest gogo build rules so we can finally get the latest gogo bits
with a bug fix I've been waiting for.

- Get latest grpc release which solves some spurious output I see on
shutdown during tests.

* Update our config protos to leverage gogo codegen options.


Former-commit-id: af2e1b3be54cdf3d32d4b5e7849ea01a8a9ae43d
  • Loading branch information
geeknoid committed Apr 3, 2017
1 parent aaddc43 commit 72672ac
Show file tree
Hide file tree
Showing 41 changed files with 202 additions and 116 deletions.
4 changes: 2 additions & 2 deletions mixer/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_repositories()

git_repository(
name = "org_pubref_rules_protobuf",
commit = "d42e895387c658eda90276aea018056fcdcb30e4", # Mar 07 2017 (gogo* support)
commit = "9ede1dbc38f0b89ae6cd8e206a22dd93cc1d5637", # Mar 31 2017 (gogo* support)
remote = "https://github.com/pubref/rules_protobuf",
)

Expand Down Expand Up @@ -114,7 +114,7 @@ new_git_repository(

new_go_repository(
name = "org_golang_google_grpc",
commit = "708a7f9f3283aa2d4f6132d287d78683babe55c8", # Dec 5, 2016 (v1.0.5)
commit = "cdee119ee21e61eef7093a41ba148fa83585e143", # Mar 14, 2017 (v1.2.0)
importpath = "google.golang.org/grpc",
)

Expand Down
4 changes: 4 additions & 0 deletions mixer/adapter/denyChecker/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,26 @@ load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogoslick_proto_library")
gogoslick_proto_library(
name = "go_default_library",
importmap = {
"gogoproto/gogo.proto": "github.com/gogo/protobuf/gogoproto",
"google/rpc/status.proto": "github.com/googleapis/googleapis/google/rpc",
},
imports = [
"../../../../external/com_github_gogo_protobuf",
"external/com_github_google_protobuf/src",
"external/com_github_googleapis_googleapis/",
],
inputs = [
"@com_github_google_protobuf//:well_known_protos",
"@com_github_googleapis_googleapis//:status_proto",
"@com_github_gogo_protobuf//gogoproto:go_default_library_protos",
],
protos = [
"config.proto",
],
verbose = 0,
visibility = ["//adapter/denyChecker:__pkg__"],
deps = [
"@com_github_gogo_protobuf//gogoproto:go_default_library",
"@com_github_googleapis_googleapis//:google/rpc",
],
)
10 changes: 7 additions & 3 deletions mixer/adapter/denyChecker/config/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ syntax = "proto3";

package adapter.denyChecker.config;

option go_package="config";

import "gogoproto/gogo.proto";
import "google/rpc/status.proto";

option go_package="config";
option (gogoproto.goproto_getters_all) = false;
option (gogoproto.equal_all) = false;
option (gogoproto.gostring_all) = false;

message Params {
// The error to return when denying a request.
google.rpc.Status error = 1;
google.rpc.Status error = 1 [(gogoproto.nullable) = false];
}
4 changes: 2 additions & 2 deletions mixer/adapter/denyChecker/denyChecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type (
var (
name = "denyChecker"
desc = "Denies every check request"
conf = &config.Params{Error: &rpc.Status{Code: int32(rpc.FAILED_PRECONDITION)}}
conf = &config.Params{Error: rpc.Status{Code: int32(rpc.FAILED_PRECONDITION)}}
)

// Register records the builders exposed by this adapter.
Expand All @@ -46,7 +46,7 @@ func (builder) NewDenialsAspect(env adapter.Env, c adapter.Config) (adapter.Deni
}

func newDenyChecker(c *config.Params) (*denier, error) {
return &denier{status: *c.Error}, nil
return &denier{status: c.Error}, nil
}

func (d *denier) Close() error { return nil }
Expand Down
2 changes: 1 addition & 1 deletion mixer/adapter/denyChecker/denyChecker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestAll(t *testing.T) {
t.Errorf("a.Close failed: %v", err)
}

a, err = b.NewDenialsAspect(nil, &config.Params{Error: &rpc.Status{Code: int32(rpc.INVALID_ARGUMENT)}})
a, err = b.NewDenialsAspect(nil, &config.Params{Error: rpc.Status{Code: int32(rpc.INVALID_ARGUMENT)}})
if err != nil {
t.Errorf("Unable to create aspect: %v", err)
}
Expand Down
14 changes: 14 additions & 0 deletions mixer/adapter/genericListChecker/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,23 @@ load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogoslick_proto_library")

gogoslick_proto_library(
name = "go_default_library",
importmap = {
"gogoproto/gogo.proto": "github.com/gogo/protobuf/gogoproto",
},
imports = [
"../../../../external/com_github_gogo_protobuf",
"external/com_github_google_protobuf/src",
],
inputs = [
"@com_github_google_protobuf//:well_known_protos",
"@com_github_gogo_protobuf//gogoproto:go_default_library_protos",
],
protos = [
"config.proto",
],
verbose = 0,
visibility = ["//adapter/genericListChecker:__pkg__"],
deps = [
"@com_github_gogo_protobuf//gogoproto:go_default_library",
],
)
5 changes: 5 additions & 0 deletions mixer/adapter/genericListChecker/config/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ syntax = "proto3";

package adapter.genericListChecker.config;

import "gogoproto/gogo.proto";

option go_package="config";
option (gogoproto.goproto_getters_all) = false;
option (gogoproto.equal_all) = false;
option (gogoproto.gostring_all) = false;

message Params {
// The set of entries in the list to check against
Expand Down
1 change: 0 additions & 1 deletion mixer/adapter/ipListChecker/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ go_library(
"//adapter/ipListChecker/config:go_default_library",
"//pkg/adapter:go_default_library",
"@com_github_ghodss_yaml//:go_default_library",
"@com_github_gogo_protobuf//types:go_default_library",
],
)

Expand Down
4 changes: 4 additions & 0 deletions mixer/adapter/ipListChecker/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogoslick_proto_library")
gogoslick_proto_library(
name = "go_default_library",
importmap = {
"gogoproto/gogo.proto": "github.com/gogo/protobuf/gogoproto",
"google/protobuf/duration.proto": "github.com/gogo/protobuf/types",
},
imports = [
"../../../../external/com_github_gogo_protobuf",
"external/com_github_google_protobuf/src",
],
inputs = [
"@com_github_gogo_protobuf//gogoproto:go_default_library_protos",
"@com_github_google_protobuf//:well_known_protos",
],
protos = [
Expand All @@ -19,6 +22,7 @@ gogoslick_proto_library(
verbose = 0,
visibility = ["//adapter/ipListChecker:__pkg__"],
deps = [
"@com_github_gogo_protobuf//gogoproto:go_default_library",
"@com_github_gogo_protobuf//types:go_default_library",
],
)
8 changes: 6 additions & 2 deletions mixer/adapter/ipListChecker/config/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,24 @@ syntax = "proto3";
package adapter.ipListChecker.config;

import "google/protobuf/duration.proto";
import "gogoproto/gogo.proto";

option go_package="config";
option (gogoproto.goproto_getters_all) = false;
option (gogoproto.equal_all) = false;
option (gogoproto.gostring_all) = false;

message Params {
// Where to find the list to check against
string provider_url = 1;

// Determines how often the provider is polled for
// an updated list
google.protobuf.Duration refresh_interval = 2;
google.protobuf.Duration refresh_interval = 2 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];

// Indicates how long to keep a list before discarding it.
// Typically, the TTL value should be set to noticeably longer (> 2x) than the
// refresh interval to ensure continued operation in the face of transient
// server outages.
google.protobuf.Duration ttl = 3;
google.protobuf.Duration ttl = 3 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
}
26 changes: 7 additions & 19 deletions mixer/adapter/ipListChecker/ipListChecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"time"

"github.com/ghodss/yaml"
ptypes "github.com/gogo/protobuf/types"

"istio.io/mixer/adapter/ipListChecker/config"
"istio.io/mixer/pkg/adapter"
Expand Down Expand Up @@ -59,8 +58,8 @@ var (
desc = "Checks whether an IP address is present in an IP address list."
conf = &config.Params{
ProviderUrl: "http://localhost",
RefreshInterval: &ptypes.Duration{Seconds: 60},
Ttl: &ptypes.Duration{Seconds: 300},
RefreshInterval: time.Duration(60) * time.Second,
Ttl: time.Duration(300) * time.Second,
}
)

Expand Down Expand Up @@ -89,30 +88,19 @@ func (builder) ValidateConfig(cfg adapter.Config) (ce *adapter.ConfigErrors) {
}
}

refresh, err := ptypes.DurationFromProto(c.RefreshInterval)
if err != nil {
ce = ce.Append("RefreshInterval", err)
} else if refresh <= 0 {
ce = ce.Appendf("RefreshInterval", "refresh interval must be at least 1 second, it is %v", refresh)
if c.RefreshInterval <= 0 {
ce = ce.Appendf("RefreshInterval", "refresh interval must be at least 1 second, it is %v", c.RefreshInterval)
}

ttl, err := ptypes.DurationFromProto(c.Ttl)
if err != nil {
ce = ce.Append("Ttl", err)
return
}
if ttl < refresh {
ce = ce.Appendf("Ttl", "Ttl must be > RefreshInterval, Ttl is %v and RefreshInterval is %v", ttl, refresh)
if c.Ttl < c.RefreshInterval {
ce = ce.Appendf("Ttl", "Ttl must be > RefreshInterval, Ttl is %v and RefreshInterval is %v", c.Ttl, c.RefreshInterval)
}

return
}

func newListChecker(env adapter.Env, c *config.Params) (*listChecker, error) {
refresh, _ := ptypes.DurationFromProto(c.RefreshInterval)
ttl, _ := ptypes.DurationFromProto(c.Ttl)

return newListCheckerWithTimers(env, c, time.NewTicker(refresh), time.NewTimer(ttl), ttl)
return newListCheckerWithTimers(env, c, time.NewTicker(c.RefreshInterval), time.NewTimer(c.Ttl), c.Ttl)
}

func newListCheckerWithTimers(env adapter.Env, c *config.Params,
Expand Down
11 changes: 5 additions & 6 deletions mixer/adapter/ipListChecker/ipListChecker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"time"

"github.com/ghodss/yaml"
ptypes "github.com/gogo/protobuf/types"

"istio.io/mixer/adapter/ipListChecker/config"
"istio.io/mixer/pkg/adapter"
Expand Down Expand Up @@ -216,17 +215,17 @@ func TestValidateConfig(t *testing.T) {
},

{
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: &ptypes.Duration{Seconds: -1, Nanos: -1}, Ttl: toDuration(2)},
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: toDuration(-1), Ttl: toDuration(2)},
field: "RefreshInterval",
},

{
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: &ptypes.Duration{Seconds: 0x7fffffffffffffff, Nanos: -1}, Ttl: toDuration(2)},
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: toDuration(0x7fffffffffffffff), Ttl: toDuration(2)},
field: "RefreshInterval",
},

{
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: toDuration(1), Ttl: &ptypes.Duration{Seconds: 0x7fffffffffffffff, Nanos: -1}},
cfg: config.Params{ProviderUrl: "http://foo.com", RefreshInterval: toDuration(1), Ttl: toDuration(0x7fffffffffffffff)},
field: "Ttl",
},

Expand Down Expand Up @@ -331,6 +330,6 @@ func TestInvariants(t *testing.T) {
test.AdapterInvariants(Register, t)
}

func toDuration(seconds int32) *ptypes.Duration {
return ptypes.DurationProto(time.Duration(seconds) * time.Second)
func toDuration(seconds int64) time.Duration {
return time.Duration(seconds) * time.Second
}
1 change: 0 additions & 1 deletion mixer/adapter/memQuota/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ go_library(
"//adapter/memQuota/util:go_default_library",
"//pkg/adapter:go_default_library",
"//pkg/pool:go_default_library",
"@com_github_gogo_protobuf//types:go_default_library",
],
)

Expand Down
4 changes: 4 additions & 0 deletions mixer/adapter/memQuota/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,23 @@ gogoslick_proto_library(
name = "go_default_library",
importmap = {
"google/protobuf/duration.proto": "github.com/gogo/protobuf/types",
"gogoproto/gogo.proto": "github.com/gogo/protobuf/gogoproto",
},
imports = [
"../../../../external/com_github_gogo_protobuf",
"external/com_github_google_protobuf/src",
],
inputs = [
"@com_github_google_protobuf//:well_known_protos",
"@com_github_gogo_protobuf//gogoproto:go_default_library_protos",
],
protos = [
"config.proto",
],
verbose = 0,
visibility = ["//adapter/memQuota:__pkg__"],
deps = [
"@com_github_gogo_protobuf//gogoproto:go_default_library",
"@com_github_gogo_protobuf//types:go_default_library",
],
)
6 changes: 5 additions & 1 deletion mixer/adapter/memQuota/config/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ syntax = "proto3";
package adapter.memQuota.config;

import "google/protobuf/duration.proto";
import "gogoproto/gogo.proto";

option go_package="config";
option (gogoproto.goproto_getters_all) = false;
option (gogoproto.equal_all) = false;
option (gogoproto.gostring_all) = false;

message Params {
// Minimum number of seconds that deduplication is possible for a given operation.
google.protobuf.Duration min_deduplication_duration = 1;
google.protobuf.Duration min_deduplication_duration = 1 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
}
17 changes: 4 additions & 13 deletions mixer/adapter/memQuota/memQuota.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ package memQuota
import (
"time"

ptypes "github.com/gogo/protobuf/types"

"istio.io/mixer/adapter/memQuota/config"
"istio.io/mixer/adapter/memQuota/util"
"istio.io/mixer/pkg/adapter"
Expand All @@ -53,7 +51,7 @@ var (
name = "memQuota"
desc = "Simple volatile memory-based quotas."
conf = &config.Params{
MinDeduplicationDuration: &ptypes.Duration{Seconds: 1},
MinDeduplicationDuration: time.Duration(1) * time.Second,
}
)

Expand All @@ -69,13 +67,8 @@ func newBuilder() builder {
func (builder) ValidateConfig(cfg adapter.Config) (ce *adapter.ConfigErrors) {
c := cfg.(*config.Params)

dedupWindow, err := ptypes.DurationFromProto(c.MinDeduplicationDuration)
if err != nil {
ce = ce.Append("MinDeduplicationDuration", err)
return
}
if dedupWindow <= 0 {
ce = ce.Appendf("MinDeduplicationDuration", "deduplication window of %v is invalid, must be > 0", dedupWindow)
if c.MinDeduplicationDuration <= 0 {
ce = ce.Appendf("MinDeduplicationDuration", "deduplication window of %v is invalid, must be > 0", c.MinDeduplicationDuration)
}
return
}
Expand All @@ -86,9 +79,7 @@ func (builder) NewQuotasAspect(env adapter.Env, c adapter.Config, d map[string]*

// newAspect returns a new aspect.
func newAspect(env adapter.Env, c *config.Params) (adapter.QuotasAspect, error) {
dedupWindow, _ := ptypes.DurationFromProto(c.MinDeduplicationDuration)

return newAspectWithDedup(env, time.NewTicker(dedupWindow))
return newAspectWithDedup(env, time.NewTicker(c.MinDeduplicationDuration))
}

// newAspect returns a new aspect.
Expand Down
Loading

0 comments on commit 72672ac

Please sign in to comment.