-
Notifications
You must be signed in to change notification settings - Fork 139
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
- Loading branch information
Luke Sneeringer
committed
Jun 27, 2020
1 parent
c3ac53d
commit 5383505
Showing
11 changed files
with
474 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
--- | ||
aip_listing: 124 | ||
permalink: /124/ | ||
redirect_from: | ||
- /0124/ | ||
--- | ||
|
||
# Resource association | ||
|
||
{% include linter-aip-listing.md aip=124 %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
--- | ||
rule: | ||
aip: 124 | ||
name: [core, '0124', valid-reference] | ||
summary: Resource patterns should use consistent variable naming. | ||
permalink: /124/valid-reference | ||
redirect_from: | ||
- /0124/valid-reference | ||
--- | ||
|
||
# Valid resource references | ||
|
||
This rule enforces that resource reference annotations refer to valid and | ||
reachable resource types, as described in [AIP-124][]. | ||
|
||
## Details | ||
|
||
This rule scans all fields with `google.api.resource_reference` annotations, | ||
and complains if the `type` on them refers to a resource with no corresponding | ||
`google.api.resource` or `google.api.resource_definition`. | ||
|
||
The rule scans the file where the field is found and all files imported by that | ||
file (recursively) as long as they are in the same package. | ||
|
||
Certain common resource types are exempt from this rule. | ||
|
||
## Examples | ||
|
||
**Incorrect** code for this rule: | ||
|
||
```proto | ||
// Incorrect. | ||
message Book { | ||
// Needs a resource annotation; without one, resource references are invalid. | ||
string name = 1; | ||
// ... | ||
} | ||
message GetBookRequest { | ||
string name = 1 [(google.api.resource_reference) = { | ||
type: "library.googleapis.com/Book" // Lint warning; referennce not found. | ||
}] | ||
} | ||
``` | ||
|
||
**Correct** code for this rule: | ||
|
||
```proto | ||
// Correct. | ||
message Book { | ||
option (google.api.resource) = { | ||
type: "library.googleapis.com/Book" | ||
pattern: "publishers/{publisher}/books/{book}" | ||
}; | ||
string name = 1; | ||
// ... | ||
} | ||
message GetBookRequest { | ||
string name = 1 [(google.api.resource_reference) = { | ||
type: "library.googleapis.com/Book" | ||
}] | ||
} | ||
``` | ||
|
||
## Disabling | ||
|
||
If you need to violate this rule, use a leading comment above the field. | ||
Remember to also include an [aip.dev/not-precedent][] comment explaining why. | ||
|
||
```proto | ||
message Book { | ||
string name = 1; | ||
} | ||
message GetBookRequest { | ||
// (-- api-linter: core::0124::valid-reference=disabled | ||
// aip.dev/not-precedent: We need to do this because reasons. --) | ||
string name = 1 [(google.api.resource_reference) = { | ||
type: "library.googleapis.com/Book" | ||
}] | ||
} | ||
``` | ||
|
||
If you need to violate this rule for an entire file, place the comment at the | ||
top of the file. | ||
|
||
[aip-124]: http://aip.dev/124 | ||
[aip.dev/not-precedent]: https://aip.dev/not-precedent | ||
|
||
``` | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// https://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
// Package aip0124 contains rules defined in https://aip.dev/124. | ||
package aip0124 | ||
|
||
import ( | ||
"github.com/googleapis/api-linter/lint" | ||
) | ||
|
||
// AddRules accepts a register function and registers each of | ||
// this AIP's rules to it. | ||
func AddRules(r lint.RuleRegistry) error { | ||
return r.Register( | ||
124, | ||
validReference, | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// https://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package aip0124 | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/googleapis/api-linter/lint" | ||
) | ||
|
||
func TestAddRules(t *testing.T) { | ||
if err := AddRules(lint.NewRuleRegistry()); err != nil { | ||
t.Errorf("AddRules got an error: %v", err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// https://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package aip0124 | ||
|
||
import ( | ||
"fmt" | ||
|
||
"bitbucket.org/creachadair/stringset" | ||
"github.com/googleapis/api-linter/lint" | ||
"github.com/googleapis/api-linter/locations" | ||
"github.com/googleapis/api-linter/rules/internal/utils" | ||
"github.com/jhump/protoreflect/desc" | ||
) | ||
|
||
var validReference = &lint.FieldRule{ | ||
Name: lint.NewRuleName(124, "valid-reference"), | ||
OnlyIf: func(f *desc.FieldDescriptor) bool { | ||
if ref := utils.GetResourceReference(f); ref != nil { | ||
urt := ref.GetType() | ||
if urt == "" { | ||
urt = ref.GetChildType() | ||
} | ||
return !stringset.New( | ||
// Whitelist the common resource types in GCP. | ||
// FIXME: Modularize this. | ||
"cloudresourcemanager.googleapis.com/Project", | ||
"cloudresourcemanager.googleapis.com/Organization", | ||
"cloudresourcemanager.googleapis.com/Folder", | ||
"billing.googleapis.com/BillingAccount", | ||
"locations.googleapis.com/Location", | ||
|
||
// If no type is declared, ignore this. | ||
"", | ||
).Contains(urt) | ||
} | ||
return false | ||
}, | ||
LintField: func(f *desc.FieldDescriptor) []lint.Problem { | ||
// Get the type we are checking for. | ||
ref := utils.GetResourceReference(f) | ||
urt := ref.GetType() | ||
if urt == "" { | ||
urt = ref.GetChildType() | ||
} | ||
|
||
// Iterate over each dependency file and check for a matching resource. | ||
for _, file := range getDependencies(f.GetFile(), f.GetFile().GetPackage()) { | ||
// Most resources will be messages. If we find a message with a | ||
// resource annotation matching our universal resource type, we are done. | ||
for _, message := range file.GetMessageTypes() { | ||
if res := utils.GetResource(message); res != nil { | ||
if res.GetType() == urt { | ||
return nil | ||
} | ||
} | ||
} | ||
|
||
// Some resources are defined as file annotations. Check for these too. | ||
for _, rd := range utils.GetResourceDefinitions(file) { | ||
if rd.GetType() == urt { | ||
return nil | ||
} | ||
} | ||
} | ||
|
||
// We could not find a resource with that type. Return a problem. | ||
return []lint.Problem{{ | ||
Message: fmt.Sprintf("Could not find resource of type %q", urt), | ||
Descriptor: f, | ||
Location: locations.FieldResourceReference(f), | ||
}} | ||
}, | ||
} | ||
|
||
// getDependencies returns all dependencies in the same package. | ||
func getDependencies(file *desc.FileDescriptor, pkg string) map[string]*desc.FileDescriptor { | ||
answer := map[string]*desc.FileDescriptor{file.GetName(): file} | ||
for _, f := range file.GetDependencies() { | ||
if _, found := answer[f.GetName()]; !found && f.GetPackage() == pkg { | ||
answer[f.GetName()] = f | ||
for name, f2 := range getDependencies(f, pkg) { | ||
answer[name] = f2 | ||
} | ||
} | ||
} | ||
return answer | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// https://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package aip0124 | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/googleapis/api-linter/rules/internal/testutils" | ||
) | ||
|
||
func TestValidReference(t *testing.T) { | ||
for _, test := range []struct { | ||
name string | ||
Field string | ||
Ref string | ||
problems testutils.Problems | ||
}{ | ||
{"Found", "type", "library.googleapis.com/Book", nil}, | ||
{"FoundChild", "child_type", "library.googleapis.com/Book", nil}, | ||
{"NotFound", "type", "library.googleapis.com/Bok", testutils.Problems{{Message: "Could not find"}}}, | ||
{"NotFoundChild", "child_type", "library.googleapis.com/Bok", testutils.Problems{{Message: "Could not find"}}}, | ||
{"Ignored", "type", "cloudresourcemanager.googleapis.com/Project", nil}, | ||
} { | ||
t.Run(test.name, func(t *testing.T) { | ||
t.Run("SameFileResourceDefinition", func(t *testing.T) { | ||
f := testutils.ParseProto3Tmpl(t, ` | ||
import "google/api/resource.proto"; | ||
option (google.api.resource_definition) = { | ||
type: "library.googleapis.com/Book" | ||
}; | ||
message Foo { | ||
string book = 1 [(google.api.resource_reference).{{.Field}} = "{{.Ref}}"]; | ||
string irrelevant = 2; | ||
} | ||
`, test) | ||
field := f.GetMessageTypes()[0].GetFields()[0] | ||
if diff := test.problems.SetDescriptor(field).Diff(validReference.Lint(f)); diff != "" { | ||
t.Errorf(diff) | ||
} | ||
}) | ||
|
||
t.Run("SameFileResourceMessage", func(t *testing.T) { | ||
f := testutils.ParseProto3Tmpl(t, ` | ||
import "google/api/resource.proto"; | ||
message Book { | ||
option (google.api.resource) = { | ||
type: "library.googleapis.com/Book" | ||
}; | ||
} | ||
message Foo { | ||
string book = 1 [(google.api.resource_reference).{{.Field}} = "{{.Ref}}"]; | ||
} | ||
`, test) | ||
field := f.GetMessageTypes()[1].GetFields()[0] | ||
if diff := test.problems.SetDescriptor(field).Diff(validReference.Lint(f)); diff != "" { | ||
t.Errorf(diff) | ||
} | ||
}) | ||
}) | ||
|
||
t.Run("DirectDependency", func(t *testing.T) { | ||
files := testutils.ParseProto3Tmpls(t, map[string]string{ | ||
"dep.proto": ` | ||
import "google/api/resource.proto"; | ||
message Book { | ||
option (google.api.resource) = { | ||
type: "library.googleapis.com/Book" | ||
}; | ||
} | ||
`, | ||
"leaf.proto": ` | ||
import "google/api/resource.proto"; | ||
import "dep.proto"; | ||
message Foo { | ||
string book = 1 [(google.api.resource_reference).{{.Field}} = "{{.Ref}}"]; | ||
} | ||
`, | ||
}, test) | ||
file := files["leaf.proto"] | ||
field := file.GetMessageTypes()[0].GetFields()[0] | ||
if diff := test.problems.SetDescriptor(field).Diff(validReference.Lint(file)); diff != "" { | ||
t.Errorf(diff) | ||
} | ||
}) | ||
|
||
t.Run("RemoteDependency", func(t *testing.T) { | ||
files := testutils.ParseProto3Tmpls(t, map[string]string{ | ||
"dep.proto": ` | ||
import "google/api/resource.proto"; | ||
message Book { | ||
option (google.api.resource) = { | ||
type: "library.googleapis.com/Book" | ||
}; | ||
} | ||
`, | ||
"intermediate.proto": `import "dep.proto";`, | ||
"leaf.proto": ` | ||
import "google/api/resource.proto"; | ||
import "intermediate.proto"; | ||
message Foo { | ||
string book = 1 [(google.api.resource_reference).{{.Field}} = "{{.Ref}}"]; | ||
} | ||
`, | ||
}, test) | ||
file := files["leaf.proto"] | ||
field := file.GetMessageTypes()[0].GetFields()[0] | ||
if diff := test.problems.SetDescriptor(field).Diff(validReference.Lint(file)); diff != "" { | ||
t.Errorf(diff) | ||
} | ||
}) | ||
} | ||
} |
Oops, something went wrong.