Skip to content

Commit

Permalink
chore: add API deprecations mechanism
Browse files Browse the repository at this point in the history
Refs siderolabs#4576.

Signed-off-by: Alexey Palazhchenko <alexey.palazhchenko@talos-systems.com>
  • Loading branch information
AlekSi committed Nov 29, 2021
1 parent bf4c81e commit d217552
Show file tree
Hide file tree
Showing 10 changed files with 1,949 additions and 2,509 deletions.
36 changes: 36 additions & 0 deletions api/common/common.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 6 additions & 23 deletions api/machine/machine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -955,13 +935,16 @@ message InstallConfig {

message MachineConfig {
enum MachineType {
option allow_alias = true;
option allow_alias = true; // TODO remove in 0.15
TYPE_UNKNOWN = 0;
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;
Expand Down
2 changes: 1 addition & 1 deletion api/prototool.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ lint:
- COMMENTS_NO_INLINE # Verifies that there are no inline comments.
# - ENUMS_HAVE_COMMENTS # Verifies that all enums have a comment of the form "// EnumName ...".
# - ENUMS_HAVE_SENTENCE_COMMENTS # Verifies that all enums have a comment that contains at least one complete sentence.
# - ENUMS_NO_ALLOW_ALIAS # Verifies that no enums use the option "allow_alias".
- ENUMS_NO_ALLOW_ALIAS # Verifies that no enums use the option "allow_alias".
# - ENUM_FIELDS_HAVE_COMMENTS # Verifies that all enum fields have a comment of the form "// FIELD_NAME ...".
# - ENUM_FIELDS_HAVE_SENTENCE_COMMENTS # Verifies that all enum fields have a comment that contains at least one complete sentence.
- ENUM_FIELD_NAMES_UPPERCASE # Verifies that all enum field names are UPPERCASE.
Expand Down
2 changes: 1 addition & 1 deletion hack/protoc-gen-doc/markdown.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
138 changes: 136 additions & 2 deletions internal/app/apid/pkg/backend/apid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -244,3 +249,132 @@ func TestAPIs(t *testing.T) {
}
}
}

func testDeprecated(t *testing.T, descriptor protoreflect.Descriptor, currentVersion *config.VersionContract) {
var deprecatedOpt bool
var deprecatedVersion string
switch opts := descriptor.Options().(type) {
case *descriptorpb.EnumOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedEnum).(string)
}
case *descriptorpb.EnumValueOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedEnumValue).(string)
}
case *descriptorpb.MessageOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedMessage).(string)
}
case *descriptorpb.FieldOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedField).(string)
}
case *descriptorpb.ServiceOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedService).(string)
}
case *descriptorpb.MethodOptions:
if opts != nil {
deprecatedOpt = pointer.GetBool(opts.Deprecated)
deprecatedVersion = protobuf.GetExtension(opts, common.E_RemoveDeprecatedMethod).(string)
}

default:
t.Fatalf("unhandled %T", opts)
}

assert.Equal(t, deprecatedOpt, deprecatedVersion != "",
"%s: `deprecated` and `remove_deprecated_XXX_in` options should be used together", descriptor.FullName())
if !deprecatedOpt || deprecatedVersion == "" {
return
}

v, err := config.ParseContractFromVersion(deprecatedVersion)
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)
}
}
}
}
Loading

0 comments on commit d217552

Please sign in to comment.