Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

s2a: Change S2A proto package name #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

s2a: Change S2A proto package name #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions grpc/gcp/s2a/proto/v2/common.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright 2024 The gRPC Authors
//
// 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.

// The canonical version of this proto can be found at
// https://github.com/grpc/grpc-proto/blob/master/grpc/gcp/s2a/common.proto

syntax = "proto3";

package s2a.proto.v2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The folder structure needs to match the package name, so this would live in the top-level folder s2a.


option java_multiple_files = true;
option java_outer_classname = "CommonProto";
option java_package = "io.grpc.s2a.internal.handshaker";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conflicts with the internal proto. I think we need to have a better undestanding of the plan.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out Eric. From our email thread about this proto package name change, and what you pointed out about conflicting java package names, my understanding is there are a few steps:

  • introduce a new proto here, with the package name s2a.proto.v2, with a java package name of io.grpc.s2a.internal.proto (or something else).
  • once this is merged, delete the protos in gRPC-Java and replace them with these protos (with a directory structure: s2a/src/main/proto/grpc/gcp/s2a/proto/v2/). This will require importing the new protos from package io.grpc.s2a.internal.proto in implementation files (currently the implementation can use protos without importing them because they are in the same java package)
  • delete the old protos in this repo
  • delete the old protos in gRPC-Java

It would probably be preferable to have the protos continue to live in the same java package as the implementation (@matthewstevenson88, please let me know if it doesn't matter) , in this case, I think we would need to take a different sequence of steps:

  • update protos in gRPC-Java to have the package name s2a.proto.v2
  • send out this PR which creates the new protos with package name s2a.proto.v2
  • in a followup PR delete the old protos in this package

However I feel like the second process I mentioned might not be feasible, since IIUC, new protos need to be added in this repo before they can be updated in gRPC-Java.

WDYT? If the first sequence is ok with you and @matthewstevenson88 , we can go ahead and implement it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about something completely different here. You can't have two copies of the same proto with different java packages without strong caution. This proto already exists internally and doesn't look like this. We need to talk about what version will win and how that will be done.

It's also against conventions to have a proto named s2a.proto and then use the Java package io.grpc.s2a. You'd normally use com.s2a or the like. We can do some funny business here, but the entire world uses a single name. And thus needing to figure out the relationship with the protos inside Google.


// The TLS 1.0-1.2 ciphersuites that the application can negotiate when using
// S2A.
enum Ciphersuite {
CIPHERSUITE_UNSPECIFIED = 0;
CIPHERSUITE_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 = 1;
CIPHERSUITE_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 = 2;
CIPHERSUITE_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 = 3;
CIPHERSUITE_ECDHE_RSA_WITH_AES_128_GCM_SHA256 = 4;
CIPHERSUITE_ECDHE_RSA_WITH_AES_256_GCM_SHA384 = 5;
CIPHERSUITE_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 = 6;
}

// The TLS versions supported by S2A's handshaker module.
enum TLSVersion {
TLS_VERSION_UNSPECIFIED = 0;
TLS_VERSION_1_0 = 1;
TLS_VERSION_1_1 = 2;
TLS_VERSION_1_2 = 3;
TLS_VERSION_1_3 = 4;
}

// The side in the TLS connection.
enum ConnectionSide {
CONNECTION_SIDE_UNSPECIFIED = 0;
CONNECTION_SIDE_CLIENT = 1;
CONNECTION_SIDE_SERVER = 2;
}

// The ALPN protocols that the application can negotiate during a TLS handshake.
enum AlpnProtocol {
ALPN_PROTOCOL_UNSPECIFIED = 0;
ALPN_PROTOCOL_GRPC = 1;
ALPN_PROTOCOL_HTTP2 = 2;
ALPN_PROTOCOL_HTTP1_1 = 3;
}

message Identity {
oneof identity_oneof {
// The SPIFFE ID of a connection endpoint.
string spiffe_id = 1;

// The hostname of a connection endpoint.
string hostname = 2;

// The UID of a connection endpoint.
string uid = 4;

// The username of a connection endpoint.
string username = 5;

// The GCP ID of a connection endpoint.
string gcp_id = 6;
}

// Additional identity-specific attributes.
map<string, string> attributes = 3;
}
Loading
Loading