Skip to content

Commit f14ae10

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 f14ae10

File tree

5 files changed

+72
-0
lines changed

5 files changed

+72
-0
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ var (
6565
"google.api.http": googleann.E_Http,
6666
}
6767
requiredMethodOptionExtensions = []string{"qdrant.cloud.common.v1.permissions", "google.api.http"}
68+
requiresAuthenticationOption = commonv1.E_RequiresAuthentication
6869
)
6970

7071
func main() {
@@ -92,6 +93,16 @@ func checkMethodOptions(ctx context.Context, responseWriter check.ResponseWriter
9293
return nil
9394
}
9495
if !proto.HasExtension(options, extension) {
96+
// special case for "qdrant.cloud.common.v1.permissions": in case
97+
// there is "qdrant.cloud.common.v1.requires_authentication" set to
98+
// false, setting permissions isn't needed.
99+
if extensionKey == "qdrant.cloud.common.v1.permissions" && proto.HasExtension(options, requiresAuthenticationOption) {
100+
val := proto.GetExtension(options, requiresAuthenticationOption).(bool)
101+
if !val {
102+
// requires_authentication is false, we skip it.
103+
break
104+
}
105+
}
95106
responseWriter.AddAnnotation(
96107
check.WithMessagef("Method %q does not define the %q option", methodDescriptor.FullName(), extension.TypeDescriptor().FullName()),
97108
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)