Skip to content

Commit b57c911

Browse files
committed
Update buf-plugin-method-options handling special case
The behavior of the "qdrant.cloud.common.v1.permissions" option depends on another option "qdrant.cloud.common.requires_authentication". With this change, we handle this special case: When `requires_authentication` is set to false, we don't validate the presence of the `permissions` option.
1 parent c8a6e91 commit b57c911

File tree

5 files changed

+81
-3
lines changed

5 files changed

+81
-3
lines changed

cmd/buf-plugin-method-options/main.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,18 @@ var (
6060
LicenseURL: "",
6161
},
6262
}
63+
permissionsOption = commonv1.E_Permissions
64+
restHTTPOption = googleann.E_Http
65+
requiresAuthenticationOption = commonv1.E_RequiresAuthentication
66+
6367
extensionRegistry = map[string]*protoimpl.ExtensionInfo{
64-
"qdrant.cloud.common.v1.permissions": commonv1.E_Permissions,
65-
"google.api.http": googleann.E_Http,
68+
string(permissionsOption.TypeDescriptor().Descriptor().FullName()): permissionsOption,
69+
string(restHTTPOption.TypeDescriptor().Descriptor().FullName()): restHTTPOption,
70+
}
71+
requiredMethodOptionExtensions = []string{
72+
string(permissionsOption.TypeDescriptor().Descriptor().FullName()),
73+
string(restHTTPOption.TypeDescriptor().Descriptor().FullName()),
6674
}
67-
requiredMethodOptionExtensions = []string{"qdrant.cloud.common.v1.permissions", "google.api.http"}
6875
)
6976

7077
func main() {
@@ -92,6 +99,16 @@ func checkMethodOptions(ctx context.Context, responseWriter check.ResponseWriter
9299
return nil
93100
}
94101
if !proto.HasExtension(options, extension) {
102+
// special case for "qdrant.cloud.common.v1.permissions": in case
103+
// there is "qdrant.cloud.common.v1.requires_authentication" set to
104+
// false, setting permissions isn't needed.
105+
if extensionKey == "qdrant.cloud.common.v1.permissions" && proto.HasExtension(options, requiresAuthenticationOption) {
106+
val := proto.GetExtension(options, requiresAuthenticationOption).(bool)
107+
if !val {
108+
// requires_authentication is false, we skip it.
109+
break
110+
}
111+
}
95112
responseWriter.AddAnnotation(
96113
check.WithMessagef("Method %q does not define the %q option", methodDescriptor.FullName(), extension.TypeDescriptor().FullName()),
97114
check.WithDescriptor(methodDescriptor),

cmd/buf-plugin-method-options/main_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,28 @@ func TestSimpleFailure(t *testing.T) {
5959
EndColumn: 5,
6060
},
6161
},
62+
{
63+
RuleID: methodOptionsRuleID,
64+
Message: "Method \"simple.GreeterService.ClosedGoodbye\" does not define the \"google.api.http\" option",
65+
FileLocation: &checktest.ExpectedFileLocation{
66+
FileName: "simple.proto",
67+
StartLine: 14,
68+
StartColumn: 4,
69+
EndLine: 18,
70+
EndColumn: 5,
71+
},
72+
},
73+
{
74+
RuleID: methodOptionsRuleID,
75+
Message: "Method \"simple.GreeterService.ClosedGoodbye\" does not define the \"qdrant.cloud.common.v1.permissions\" option",
76+
FileLocation: &checktest.ExpectedFileLocation{
77+
FileName: "simple.proto",
78+
StartLine: 14,
79+
StartColumn: 4,
80+
EndLine: 18,
81+
EndColumn: 5,
82+
},
83+
},
6284
},
6385
}.Run(t)
6486
}
@@ -81,6 +103,10 @@ func TestSimpleFailureWithOption(t *testing.T) {
81103
RuleID: methodOptionsRuleID,
82104
Message: "extension key \"unknown.extension\" does not exist",
83105
},
106+
{
107+
RuleID: methodOptionsRuleID,
108+
Message: "extension key \"unknown.extension\" does not exist",
109+
},
84110
{
85111
RuleID: methodOptionsRuleID,
86112
Message: "Method \"simple.GreeterService.HelloWorld\" does not define the \"qdrant.cloud.common.v1.permissions\" option",
@@ -92,6 +118,17 @@ func TestSimpleFailureWithOption(t *testing.T) {
92118
EndColumn: 5,
93119
},
94120
},
121+
{
122+
RuleID: methodOptionsRuleID,
123+
Message: "Method \"simple.GreeterService.ClosedGoodbye\" does not define the \"qdrant.cloud.common.v1.permissions\" option",
124+
FileLocation: &checktest.ExpectedFileLocation{
125+
FileName: "simple.proto",
126+
StartLine: 14,
127+
StartColumn: 4,
128+
EndLine: 18,
129+
EndColumn: 5,
130+
},
131+
},
95132
},
96133
}.Run(t)
97134

@@ -115,6 +152,10 @@ func TestSimpleFailureWithOptionWrongKey(t *testing.T) {
115152
RuleID: methodOptionsRuleID,
116153
Message: "extension key \"unknown.extension\" does not exist",
117154
},
155+
{
156+
RuleID: methodOptionsRuleID,
157+
Message: "extension key \"unknown.extension\" does not exist",
158+
},
118159
},
119160
}.Run(t)
120161

cmd/buf-plugin-method-options/testdata/common.proto

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,10 @@ extend google.protobuf.MethodOptions {
1111
// A list of permissions which ALL need to be met by the current user.
1212
repeated string permissions = 50001;
1313
}
14+
15+
// The extension for allowing a method to be be used without authentication.
16+
// If the extension is missing the system requires authentication and return a 'permission denied' error if missing.
17+
extend google.protobuf.MethodOptions {
18+
// Set to allow a method to be used without authentication.
19+
bool requires_authentication = 50003;
20+
}

cmd/buf-plugin-method-options/testdata/simple_failure/simple.proto

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,10 @@ service GreeterService {
1111
// missing qdrant.cloud.common.v1.permissions
1212
// missing google.api.http
1313
}
14+
15+
rpc ClosedGoodbye(google.protobuf.Empty) returns (google.protobuf.Empty) {
16+
option (qdrant.cloud.common.v1.requires_authentication) = true;
17+
// missing qdrant.cloud.common.v1.permissions
18+
// missing google.api.http
19+
}
1420
}

cmd/buf-plugin-method-options/testdata/simple_success/simple.proto

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,11 @@ service GreeterService {
1313
option (qdrant.cloud.common.v1.permissions) = "read:api_keys";
1414
option (google.api.http) = {get: "/api/hello-world"};
1515
}
16+
17+
rpc OpenGoodbye(google.protobuf.Empty) returns (google.protobuf.Empty) {
18+
// there aren't permissions required because it doesn't require
19+
// authentication.
20+
option (qdrant.cloud.common.v1.requires_authentication) = false;
21+
option (google.api.http) = {get: "/api/hello-world"};
22+
}
1623
}

0 commit comments

Comments
 (0)