chore: add API deprecations mechanism

Refs #4576.

Signed-off-by: Alexey Palazhchenko <alexey.palazhchenko@talos-systems.com>

api/common/common.proto

@@ -5,8 +5,44 @@ package common;
 option go_package = "github.com/talos-systems/talos/pkg/machinery/api/common";
 
 import "google/protobuf/any.proto";
+import "google/protobuf/descriptor.proto";
 import "google/rpc/status.proto";
 
+// An alternative to using options could be extracting versions from comments.
+// Unfortunately, they are not available: https://github.com/golang/protobuf/issues/1134
+// Also, while option numbers can be the same,
+// names should be different: https://github.com/protocolbuffers/protobuf/issues/4861
+
+extend google.protobuf.MessageOptions {
+  // Indicates the Talos version when this deprecated message will be removed from API.
+  string remove_deprecated_message = 93117;
+}
+
+extend google.protobuf.FieldOptions {
+  // Indicates the Talos version when this deprecated filed will be removed from API.
+  string remove_deprecated_field = 93117;
+}
+
+extend google.protobuf.EnumOptions {
+  // Indicates the Talos version when this deprecated enum will be removed from API.
+  string remove_deprecated_enum = 93117;
+}
+
+extend google.protobuf.EnumValueOptions {
+  // Indicates the Talos version when this deprecated enum value will be removed from API.
+  string remove_deprecated_enum_value = 93117;
+}
+
+extend google.protobuf.MethodOptions {
+  // Indicates the Talos version when this deprecated method will be removed from API.
+  string remove_deprecated_method = 93117;
+}
+
+extend google.protobuf.ServiceOptions {
+  // Indicates the Talos version when this deprecated service will be removed from API.
+  string remove_deprecated_service = 93117;
+}
+
 enum Code {
   FATAL = 0;
   LOCKED = 1;

api/machine/machine.proto

@@ -326,26 +326,6 @@ message ServiceRestartResponse {
   repeated ServiceRestart messages = 1;
 }
 
-message StartRequest {
-  option deprecated = true;
-  string id = 1;
-}
-
-message StartResponse {
-  option deprecated = true;
-  string resp = 1;
-}
-
-message StopRequest {
-  option deprecated = true;
-  string id = 1;
-}
-
-message StopResponse {
-  option deprecated = true;
-  string resp = 1;
-}
-
 // CopyRequest describes a request to copy data out of Talos node
 //
 // Copy produces .tar.gz archive which is streamed back to the caller
@@ -960,8 +940,11 @@ message MachineConfig {
     TYPE_INIT = 1;
     TYPE_CONTROL_PLANE = 2;
     TYPE_WORKER = 3;
-    // Alias for TYPE_WORKER.
-    TYPE_JOIN = 3 [deprecated = true];
+    // Deprecated alias for TYPE_WORKER. It will be removed in v0.15.
+    TYPE_JOIN = 3 [
+      (common.remove_deprecated_enum_value) = "v0.15",
+      deprecated = true
+    ];
   }
   MachineType type = 1;
   InstallConfig install_config = 2;

api/prototool.yaml

@@ -9,9 +9,6 @@ lint:
     - id: FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM
       files:
         - vendor/google/rpc/status.proto
-    - id: NAMES_NO_COMMON
-      files:
-        - common/common.proto
 
   rules:
     # The specific linters to add.
@@ -62,7 +59,7 @@ lint:
       - MESSAGE_FIELD_NAMES_NO_DESCRIPTOR                              # Verifies that all message field names are not "descriptor", which results in a collision in Java-generated code.
       - MESSAGE_NAMES_CAMEL_CASE                                       # Verifies that all non-extended message names are CamelCase.
       - MESSAGE_NAMES_CAPITALIZED                                      # Verifies that all non-extended message names are Capitalized.
-      - NAMES_NO_COMMON                                                # Verifies that no type name contains the word "common".
+      # - NAMES_NO_COMMON                                                # Verifies that no type name contains the word "common".
       # - NAMES_NO_DATA                                                  # Verifies that no type name contains the word "data".
       # - NAMES_NO_UUID                                                  # Verifies that no type name contains the word "uuid".
       - ONEOF_NAMES_LOWER_SNAKE_CASE                                   # Verifies that all oneof names are lower_snake_case.

hack/protoc-gen-doc/markdown.tmpl

@@ -15,7 +15,7 @@ description: Talos gRPC API reference.
   {{end}}
   {{- end -}}
   {{- if .Extensions }}
-  {{range .Extensions}}  - [File-level Extensions](#{{$file_name}}-extensions)
+  {{range (list .Extensions | uniq)}}  - [File-level Extensions](#{{$file_name}}-extensions)
   {{end}}
   {{- end -}}
   {{- if .Services }}

internal/app/apid/pkg/backend/apid_test.go

@@ -10,13 +10,16 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/AlekSi/pointer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/talos-systems/grpc-proxy/proxy"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/metadata"
+	protobuf "google.golang.org/protobuf/proto" //nolint:depguard,gci
 	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/types/descriptorpb"
 
 	"github.com/talos-systems/talos/internal/app/apid/pkg/backend"
 	"github.com/talos-systems/talos/pkg/grpc/middleware/authz"
@@ -25,10 +28,13 @@ import (
 	"github.com/talos-systems/talos/pkg/machinery/api/inspect"
 	"github.com/talos-systems/talos/pkg/machinery/api/machine"
 	"github.com/talos-systems/talos/pkg/machinery/api/resource"
+	"github.com/talos-systems/talos/pkg/machinery/api/security"
 	"github.com/talos-systems/talos/pkg/machinery/api/storage"
 	"github.com/talos-systems/talos/pkg/machinery/api/time"
+	"github.com/talos-systems/talos/pkg/machinery/config"
 	"github.com/talos-systems/talos/pkg/machinery/proto"
 	"github.com/talos-systems/talos/pkg/machinery/role"
+	"github.com/talos-systems/talos/pkg/version"
 )
 
 func TestAPIDInterfaces(t *testing.T) {
@@ -197,8 +203,7 @@ func TestAPIDSuite(t *testing.T) {
 	suite.Run(t, new(APIDSuite))
 }
 
-// Test idiosyncrasies of our APIs.
-func TestAPIs(t *testing.T) {
+func TestAPIIdiosyncrasies(t *testing.T) {
 	for _, services := range []protoreflect.ServiceDescriptors{
 		common.File_common_common_proto.Services(),
 		cluster.File_cluster_cluster_proto.Services(),
@@ -244,3 +249,138 @@ func TestAPIs(t *testing.T) {
 		}
 	}
 }
+
+//nolint:nakedret,gocyclo,errcheck,forcetypeassert
+func getOptions(t *testing.T, descriptor protoreflect.Descriptor) (deprecated bool, version string) {
+	switch opts := descriptor.Options().(type) {
+	case *descriptorpb.EnumOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedEnum).(string)
+		}
+	case *descriptorpb.EnumValueOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedEnumValue).(string)
+		}
+	case *descriptorpb.MessageOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedMessage).(string)
+		}
+	case *descriptorpb.FieldOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedField).(string)
+		}
+	case *descriptorpb.ServiceOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedService).(string)
+		}
+	case *descriptorpb.MethodOptions:
+		if opts != nil {
+			deprecated = pointer.GetBool(opts.Deprecated)
+			version = protobuf.GetExtension(opts, common.E_RemoveDeprecatedMethod).(string)
+		}
+
+	default:
+		t.Fatalf("unhandled %T", opts)
+	}
+
+	return
+}
+
+func testDeprecated(t *testing.T, descriptor protoreflect.Descriptor, currentVersion *config.VersionContract) {
+	deprecated, version := getOptions(t, descriptor)
+
+	assert.Equal(t, deprecated, version != "",
+		"%s: `deprecated` and `remove_deprecated_XXX_in` options should be used together", descriptor.FullName())
+
+	if !deprecated || version == "" {
+		return
+	}
+
+	v, err := config.ParseContractFromVersion(version)
+	require.NoError(t, err, "%s", descriptor.FullName())
+
+	assert.True(t, v.Greater(currentVersion), "%s should be removed in this version", descriptor.FullName())
+}
+
+func testEnum(t *testing.T, enum protoreflect.EnumDescriptor, currentVersion *config.VersionContract) {
+	testDeprecated(t, enum, currentVersion)
+
+	values := enum.Values()
+	for i := 0; i < values.Len(); i++ {
+		testDeprecated(t, values.Get(i), currentVersion)
+	}
+}
+
+func testMessage(t *testing.T, message protoreflect.MessageDescriptor, currentVersion *config.VersionContract) {
+	testDeprecated(t, message, currentVersion)
+
+	fields := message.Fields()
+	for i := 0; i < fields.Len(); i++ {
+		testDeprecated(t, fields.Get(i), currentVersion)
+	}
+
+	oneofs := message.Oneofs()
+	for i := 0; i < oneofs.Len(); i++ {
+		testDeprecated(t, oneofs.Get(i), currentVersion)
+	}
+
+	enums := message.Enums()
+	for i := 0; i < enums.Len(); i++ {
+		testEnum(t, enums.Get(i), currentVersion)
+	}
+
+	// test nested messages
+	messages := message.Messages()
+	for i := 0; i < messages.Len(); i++ {
+		testMessage(t, messages.Get(i), currentVersion)
+	}
+}
+
+func TestDeprecatedAPIs(t *testing.T) {
+	currentVersion, err := config.ParseContractFromVersion(version.Tag)
+	require.NoError(t, err)
+
+	for _, file := range []protoreflect.FileDescriptor{
+		common.File_common_common_proto,
+		cluster.File_cluster_cluster_proto,
+		inspect.File_inspect_inspect_proto,
+		machine.File_machine_machine_proto,
+		resource.File_resource_resource_proto,
+		security.File_security_security_proto,
+		storage.File_storage_storage_proto,
+		time.File_time_time_proto,
+	} {
+		enums := file.Enums()
+		for i := 0; i < enums.Len(); i++ {
+			testEnum(t, enums.Get(i), currentVersion)
+		}
+
+		messages := file.Messages()
+		for i := 0; i < messages.Len(); i++ {
+			testMessage(t, messages.Get(i), currentVersion)
+		}
+
+		services := file.Services()
+		for i := 0; i < services.Len(); i++ {
+			service := services.Get(i)
+			testDeprecated(t, service, currentVersion)
+
+			methods := service.Methods()
+			for j := 0; j < methods.Len(); j++ {
+				method := methods.Get(j)
+				testDeprecated(t, method, currentVersion)
+
+				message := method.Input()
+				testMessage(t, message, currentVersion)
+
+				message = method.Output()
+				testMessage(t, message, currentVersion)
+			}
+		}
+	}
+}