Repository: apigee/registry (go)

Issue:
Registry tool: add registry rules for ApiDeployment
Add ApiVersion resource checks to `registry check` command.

rule group | rule number | resource | field | type | severity | check
-- | -- | -- | -- | -- | -- | --
Registry | 106 | ApiDeployment | api_spec_revision | string | ERROR | valid SpecRevision name, child of parent API
Registry | 107 | ApiDeployment | endpoint_uri | string | ERROR | valid URI format
Registry | 108 | ApiDeployment | external_channel_uri | string | ERROR | valid URI format



Pull request:
add registry rules for ApiDeployment
Fixes #955

Patch:
diff --git a/cmd/registry/cmd/check/rules/rule106/rule106.go b/cmd/registry/cmd/check/rules/rule106/rule106.go
new file mode 100644
index 0000000..b987141
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule106/rule106.go
@@ -0,0 +1,84 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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.
+
+// ApiDeployment api_spec_revision must be an SpecRevision and sibling under the parent Api.
+package rule106
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/rpc"
+	"github.com/apigee/registry/server/registry/names"
+)
+
+var ruleNum = 106
+var ruleName = lint.NewRuleName(ruleNum, "deployment-api-spec-revision-ref")
+
+// AddRules accepts a register function and registers each of
+// this rules' checks to the RuleRegistry.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		ruleNum,
+		apiSpecRevisionRef,
+	)
+}
+
+var apiSpecRevisionRef = &lint.ApiDeploymentRule{
+	Name: ruleName,
+	OnlyIf: func(a *rpc.ApiDeployment) bool {
+		return strings.TrimSpace(a.ApiSpecRevision) != ""
+	},
+	ApplyToApiDeployment: func(ctx context.Context, a *rpc.ApiDeployment) []lint.Problem {
+		specName, err := names.ParseSpecRevision(a.ApiSpecRevision)
+		if err != nil {
+			return []lint.Problem{{
+				Severity:   lint.ERROR,
+				Message:    fmt.Sprintf(`api_spec_revision %q is not a valid ApiSpecRevision name.`, a.ApiSpecRevision),
+				Suggestion: fmt.Sprintf(`Parse error: %s`, err),
+			}}
+		}
+		if specName.RevisionID == "" {
+			return []lint.Problem{{
+				Severity:   lint.ERROR,
+				Message:    fmt.Sprintf(`api_spec_revision %q is not a valid ApiSpecRevision name.`, a.ApiSpecRevision),
+				Suggestion: `A revision ID is required.`,
+			}}
+		}
+
+		deploymentName, _ := names.ParseDeployment(a.Name) // name assumed to be valid
+		if specName.Api() != deploymentName.Api() {
+			return []lint.Problem{{
+				Severity:   lint.ERROR,
+				Message:    fmt.Sprintf(`api_spec_revision %q is not an API sibling of this Deployment.`, a.ApiSpecRevision),
+				Suggestion: `Correct the api_spec_revision.`,
+			}}
+		}
+
+		registryClient := lint.RegistryClient(ctx)
+		if _, err := registryClient.GetApiSpec(ctx, &rpc.GetApiSpecRequest{
+			Name: a.ApiSpecRevision,
+		}); err != nil {
+			return []lint.Problem{{
+				Severity:   lint.ERROR,
+				Message:    fmt.Sprintf(`api_spec_revision %q not found in registry.`, a.ApiSpecRevision),
+				Suggestion: `Correct the api_spec_revision.`,
+			}}
+		}
+
+		return nil
+	},
+}
diff --git a/cmd/registry/cmd/check/rules/rule107/rule107.go b/cmd/registry/cmd/check/rules/rule107/rule107.go
new file mode 100644
index 0000000..3e4408e
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule107/rule107.go
@@ -0,0 +1,45 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 rule107
+
+import (
+	"context"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/cmd/registry/cmd/check/rules/util"
+	"github.com/apigee/registry/rpc"
+)
+
+var ruleNum = 107
+var ruleName = lint.NewRuleName(ruleNum, "apideployment-endpoint-uri-format")
+
+// AddRules accepts a register function and registers each of
+// this rules' checks to the RuleRegistry.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		ruleNum,
+		endpointUriFormat,
+	)
+}
+
+var endpointUriFormat = &lint.ApiDeploymentRule{
+	Name: ruleName,
+	OnlyIf: func(a *rpc.ApiDeployment) bool {
+		return a.EndpointUri != ""
+	},
+	ApplyToApiDeployment: func(ctx context.Context, a *rpc.ApiDeployment) []lint.Problem {
+		return util.CheckURI("endpoint_uri", a.EndpointUri)
+	},
+}
diff --git a/cmd/registry/cmd/check/rules/rule108/rule108.go b/cmd/registry/cmd/check/rules/rule108/rule108.go
new file mode 100644
index 0000000..2ef4a17
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule108/rule108.go
@@ -0,0 +1,45 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 rule108
+
+import (
+	"context"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/cmd/registry/cmd/check/rules/util"
+	"github.com/apigee/registry/rpc"
+)
+
+var ruleNum = 108
+var ruleName = lint.NewRuleName(ruleNum, "apideployment-external-channel-uri-format")
+
+// AddRules accepts a register function and registers each of
+// this rules' checks to the RuleRegistry.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		ruleNum,
+		externalChannelUriFormat,
+	)
+}
+
+var externalChannelUriFormat = &lint.ApiDeploymentRule{
+	Name: ruleName,
+	OnlyIf: func(a *rpc.ApiDeployment) bool {
+		return a.EndpointUri != ""
+	},
+	ApplyToApiDeployment: func(ctx context.Context, a *rpc.ApiDeployment) []lint.Problem {
+		return util.CheckURI("external_channel_uri", a.EndpointUri)
+	},
+}
diff --git a/cmd/registry/cmd/check/rules/rules.go b/cmd/registry/cmd/check/rules/rules.go
index d81533b..2ac9fa7 100644
--- a/cmd/registry/cmd/check/rules/rules.go
+++ b/cmd/registry/cmd/check/rules/rules.go
@@ -39,6 +39,9 @@ import (
 	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule103"
 	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule104"
 	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule105"
+	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule106"
+	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule107"
+	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule108"
 	"github.com/apigee/registry/cmd/registry/cmd/check/rules/rule110"
 )
 
@@ -51,6 +54,9 @@ var addRulesFuncs = []addRulesFuncType{
 	rule103.AddRules,
 	rule104.AddRules,
 	rule105.AddRules,
+	rule106.AddRules,
+	rule107.AddRules,
+	rule108.AddRules,
 	rule110.AddRules,
 }
 
diff --git a/cmd/registry/cmd/check/rules/util/uri.go b/cmd/registry/cmd/check/rules/util/uri.go
new file mode 100644
index 0000000..44a7d61
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/util/uri.go
@@ -0,0 +1,36 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 util
+
+import (
+	"fmt"
+	"net/url"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+)
+
+func CheckURI(field, uri string) []lint.Problem {
+	if uri != "" {
+		u, err := url.ParseRequestURI(uri)
+		if err != nil || u.Host == "" {
+			return []lint.Problem{{
+				Severity:   lint.ERROR,
+				Message:    fmt.Sprintf(`%s must be an absolute URI.`, field),
+				Suggestion: fmt.Sprintf(`Ensure %s includes a host.`, field),
+			}}
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/cmd/registry/cmd/check/rules/rule106/rule106_test.go b/cmd/registry/cmd/check/rules/rule106/rule106_test.go
new file mode 100644
index 0000000..9026215
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule106/rule106_test.go
@@ -0,0 +1,99 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 rule106
+
+import (
+	"context"
+	"testing"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/pkg/connection/grpctest"
+	"github.com/apigee/registry/rpc"
+	"github.com/apigee/registry/server/registry"
+	"github.com/apigee/registry/server/registry/test/seeder"
+	"github.com/google/go-cmp/cmp"
+)
+
+// TestMain will set up a local RegistryServer and grpc.Server for all
+// tests in this package if APG_REGISTRY_ADDRESS env var is not set
+// for the client.
+func TestMain(m *testing.M) {
+	grpctest.TestMain(m, registry.Config{})
+}
+
+func TestAddRules(t *testing.T) {
+	if err := AddRules(lint.NewRuleRegistry()); err != nil {
+		t.Errorf("AddRules got an error: %v", err)
+	}
+}
+
+func Test_apiSpecRevisionRef(t *testing.T) {
+	ctx := context.Background()
+	name := "projects/check-test/locations/global/apis/myapi/versions/myversion/specs/good"
+	registryClient, _ := grpctest.SetupRegistry(ctx, t, "check-test", []seeder.RegistryResource{
+		&rpc.ApiSpec{
+			Name: name,
+		},
+	})
+	spec, err := registryClient.GetApiSpec(ctx, &rpc.GetApiSpecRequest{Name: name})
+	if err != nil {
+		t.Fatal(err)
+	}
+	revName := name + "@" + spec.RevisionId
+
+	ctx = context.WithValue(context.Background(), lint.ContextKeyRegistryClient, registryClient)
+
+	for _, tt := range []struct {
+		desc     string
+		in       string
+		expected []lint.Problem
+	}{
+		{"empty", "", nil},
+		{"unable to parse", "bad", []lint.Problem{{
+			Severity:   lint.ERROR,
+			Message:    `api_spec_revision "bad" is not a valid ApiSpecRevision name.`,
+			Suggestion: `Parse error: invalid spec revision name "bad": must match "^projects/([A-Za-z0-9-.]+)/locations/global/apis/([A-Za-z0-9-.]+)/versions/([A-Za-z0-9-.]+)/specs/([A-Za-z0-9-.]+)(?:@([a-z0-9-]+))?$"`,
+		}}},
+		{"not a revision", name, []lint.Problem{{
+			Severity:   lint.ERROR,
+			Message:    `api_spec_revision "projects/check-test/locations/global/apis/myapi/versions/myversion/specs/good" is not a valid ApiSpecRevision name.`,
+			Suggestion: `A revision ID is required.`,
+		}}},
+		{"not a sibling", "projects/check-test/locations/global/apis/bad/versions/myversion/specs/spec@foo", []lint.Problem{{
+			Severity:   lint.ERROR,
+			Message:    `api_spec_revision "projects/check-test/locations/global/apis/bad/versions/myversion/specs/spec@foo" is not an API sibling of this Deployment.`,
+			Suggestion: `Correct the api_spec_revision.`,
+		}}},
+		{"missing", name + "@missing", []lint.Problem{{
+			Severity:   lint.ERROR,
+			Message:    `api_spec_revision "projects/check-test/locations/global/apis/myapi/versions/myversion/specs/good@missing" not found in registry.`,
+			Suggestion: `Correct the api_spec_revision.`,
+		}}},
+		{"good", revName, nil},
+	} {
+		t.Run(tt.desc, func(t *testing.T) {
+			a := &rpc.ApiDeployment{
+				Name:            "projects/check-test/locations/global/apis/myapi/deployments/mydeployment",
+				ApiSpecRevision: tt.in,
+			}
+			if apiSpecRevisionRef.OnlyIf(a) {
+				got := apiSpecRevisionRef.ApplyToApiDeployment(ctx, a)
+				if diff := cmp.Diff(got, tt.expected); diff != "" {
+					t.Errorf("unexpected diff: (-want +got):\n%s", diff)
+				}
+			}
+		})
+	}
+}
diff --git a/cmd/registry/cmd/check/rules/rule107/rule107_test.go b/cmd/registry/cmd/check/rules/rule107/rule107_test.go
new file mode 100644
index 0000000..911f07f
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule107/rule107_test.go
@@ -0,0 +1,59 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 rule107
+
+import (
+	"context"
+	"testing"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/rpc"
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestAddRules(t *testing.T) {
+	if err := AddRules(lint.NewRuleRegistry()); err != nil {
+		t.Errorf("AddRules got an error: %v", err)
+	}
+}
+
+func Test_sourceUriFormat(t *testing.T) {
+	prob := []lint.Problem{{
+		Severity:   lint.ERROR,
+		Message:    `endpoint_uri must be an absolute URI.`,
+		Suggestion: `Ensure endpoint_uri includes a host.`,
+	}}
+
+	for _, tt := range []struct {
+		in       string
+		expected []lint.Problem
+	}{
+		{"", nil},
+		{"x", prob},
+		{"https://google.com/ok#yes?ok=true", nil},
+	} {
+		t.Run(tt.in, func(t *testing.T) {
+			a := &rpc.ApiDeployment{
+				EndpointUri: tt.in,
+			}
+			if endpointUriFormat.OnlyIf(a) {
+				got := endpointUriFormat.ApplyToApiDeployment(context.Background(), a)
+				if diff := cmp.Diff(got, tt.expected); diff != "" {
+					t.Errorf("unexpected diff: (-want +got):\n%s", diff)
+				}
+			}
+		})
+	}
+}
diff --git a/cmd/registry/cmd/check/rules/rule108/rule108_test.go b/cmd/registry/cmd/check/rules/rule108/rule108_test.go
new file mode 100644
index 0000000..e2efcea
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/rule108/rule108_test.go
@@ -0,0 +1,59 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 rule108
+
+import (
+	"context"
+	"testing"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/apigee/registry/rpc"
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestAddRules(t *testing.T) {
+	if err := AddRules(lint.NewRuleRegistry()); err != nil {
+		t.Errorf("AddRules got an error: %v", err)
+	}
+}
+
+func Test_externalChannelURIFormat(t *testing.T) {
+	prob := []lint.Problem{{
+		Severity:   lint.ERROR,
+		Message:    `external_channel_uri must be an absolute URI.`,
+		Suggestion: `Ensure external_channel_uri includes a host.`,
+	}}
+
+	for _, tt := range []struct {
+		in       string
+		expected []lint.Problem
+	}{
+		{"", nil},
+		{"x", prob},
+		{"https://google.com/ok#yes?ok=true", nil},
+	} {
+		t.Run(tt.in, func(t *testing.T) {
+			a := &rpc.ApiDeployment{
+				EndpointUri: tt.in,
+			}
+			if externalChannelUriFormat.OnlyIf(a) {
+				got := externalChannelUriFormat.ApplyToApiDeployment(context.Background(), a)
+				if diff := cmp.Diff(got, tt.expected); diff != "" {
+					t.Errorf("unexpected diff: (-want +got):\n%s", diff)
+				}
+			}
+		})
+	}
+}
diff --git a/cmd/registry/cmd/check/rules/util/uri_test.go b/cmd/registry/cmd/check/rules/util/uri_test.go
new file mode 100644
index 0000000..4a1e0c8
--- /dev/null
+++ b/cmd/registry/cmd/check/rules/util/uri_test.go
@@ -0,0 +1,50 @@
+// Copyright 2023 Google LLC. All Rights Reserved.
+//
+// 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
+//
+//    http://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 util
+
+import (
+	"testing"
+
+	"github.com/apigee/registry/cmd/registry/cmd/check/lint"
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestCheckURI(t *testing.T) {
+	prob := []lint.Problem{{
+		Severity:   lint.ERROR,
+		Message:    `test must be an absolute URI.`,
+		Suggestion: `Ensure test includes a host.`,
+	}}
+
+	for _, tt := range []struct {
+		in       string
+		expected []lint.Problem
+	}{
+		{"", nil},
+		{"x", prob},
+		{"not a uri", prob},
+		{"http://localhost", nil},
+		{"http://127.0.0.1", nil},
+		{"http://ok/ok", nil},
+		{"https://google.com/ok#yes?ok=true", nil},
+	} {
+		t.Run(tt.in, func(t *testing.T) {
+			got := CheckURI("test", tt.in)
+			if diff := cmp.Diff(got, tt.expected); diff != "" {
+				t.Errorf("unexpected diff: (-want +got):\n%s", diff)
+			}
+		})
+	}
+}