Fix to parse protoreflect.Message instead of parsing the value returned by v.String()

[ffjlabo]

What this PR does:

For messages for proto's custom Service options, instead of using the string obtained with v.String(), we have modified it so that the value is obtained by reflection using protoreflect.

I checked not to show the diff in the service.pb.auth.go

cd tool/codegen
docker build -t codegen:local .

gen/code:
	# NOTE: Specify a specific version temporally until the next release.
-	docker run --rm -v ${PWD}:/repo -it --entrypoint ./tool/codegen/codegen.sh ghcr.io/pipe-cd/codegen@sha256:3fd8e22eeab21bab2a2f6c1d2770b069922f4973465d57386d672574931943e8 /repo #v0.47.3-rc0-2-g462b842

+	docker run --rm -v ${PWD}:/repo -it --entrypoint ./tool/codegen/codegen.sh codegen:local /repo #v0.47.3-rc0-2-g462b842

% make gen/code                                     (git)-[fix-protoc-gen-auth-not-to-parse-string-value]
# NOTE: Specify a specific version temporally until the next release.
docker run --rm -v /Users/s14218/oss/pipe-cd/pipecd:/repo -it --entrypoint ./tool/codegen/codegen.sh codegen:local /repo #v0.47.3-rc0-2-g462b842

- pkg/model
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/apiservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/pipedservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/webservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated
...

Successfully generated all code

% git status                                        (git)-[fix-protoc-gen-auth-not-to-parse-string-value]
On branch fix-protoc-gen-auth-not-to-parse-string-value
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   Makefile
	modified:   pkg/app/piped/platformprovider/kubernetes/kubernetestest/kubernetes.mock.go
	modified:   pkg/cache/cachetest/cache.mock.go
	modified:   pkg/datastore/datastoretest/datastore.mock.go
	modified:   pkg/filestore/filestoretest/filestore.mock.go
	modified:   pkg/git/gittest/git.mock.go
	modified:   pkg/insight/insighttest/insight.mock.go
	modified:   pkg/jwt/jwttest/jwt.mock.go
	modified:   pkg/redis/redistest/redis.mock.go

Why we need it:

The output of v.String() for the proto Message type changes depending on the byte data of the compiled binary.

More detail:
The protoc-gen-auth plugin generates the restriction logic for the api request based on the role.
We can define the option in the rpc definition like below.

// WebService contains all RPC definitions for web client.
// All of these RPCs are only called by web client and authenticated by using ID_TOKEN.
service WebService {
    // Piped
    rpc RegisterPiped(RegisterPipedRequest) returns (RegisterPipedResponse) {
        option (model.rbac).resource = PIPED;
        option (model.rbac).action = CREATE;
    }

Then, generate the logic in the service.pb.auth.go.

// Authorize checks whether a role is enough for given gRPC method or not.
func (a *authorizer) Authorize(ctx context.Context, method string, r model.Role) bool {
	roles, err := a.getProjectRBACRoles(ctx, r.ProjectId, r.ProjectRbacRoles)
	if err != nil {
		a.logger.Error("failed to get rbac roles",
			zap.String("project", r.ProjectId),
			zap.Error(err),
		)
		return false
	}

	if len(roles) == 0 {
		a.logger.Error("the user does not have existing RBAC roles",
			zap.String("project", r.ProjectId),
			zap.Strings("user-roles", r.ProjectRbacRoles),
		)
		return false
	}

	verify := func(typ model.ProjectRBACResource_ResourceType, action model.ProjectRBACPolicy_Action) bool {
		for _, r := range roles {
			if r.HasPermission(typ, action) {
				return true
			}
		}
		return false
	}

	switch method {
	case "/grpc.service.webservice.WebService/GetCommand":
		return true
        ...
	case "/grpc.service.webservice.WebService/RegisterPiped":
		return verify(model.ProjectRBACResource_PIPED, model.ProjectRBACPolicy_CREATE)

pipecd/pkg/app/server/service/webservice/service.pb.auth.go

Lines 95 to 132 in b9882f7

	// Authorize checks whether a role is enough for given gRPC method or not.
	func (a *authorizer) Authorize(ctx context.Context, method string, r model.Role) bool {
	roles, err := a.getProjectRBACRoles(ctx, r.ProjectId, r.ProjectRbacRoles)
	if err != nil {
	a.logger.Error("failed to get rbac roles",
	zap.String("project", r.ProjectId),
	zap.Error(err),
	)
	return false
	}
	if len(roles) == 0 {
	a.logger.Error("the user does not have existing RBAC roles",
	zap.String("project", r.ProjectId),
	zap.Strings("user-roles", r.ProjectRbacRoles),
	)
	return false
	}
	verify := func(typ model.ProjectRBACResource_ResourceType, action model.ProjectRBACPolicy_Action) bool {
	for _, r := range roles {
	if r.HasPermission(typ, action) {
	return true
	}
	}
	return false
	}
	switch method {
	case "/grpc.service.webservice.WebService/GetCommand":
	return true
	case "/grpc.service.webservice.WebService/GenerateAPIKey":
	return verify(model.ProjectRBACResource_API_KEY, model.ProjectRBACPolicy_CREATE)
	case "/grpc.service.webservice.WebService/DisableAPIKey":
	return verify(model.ProjectRBACResource_API_KEY, model.ProjectRBACPolicy_UPDATE)
	case "/grpc.service.webservice.WebService/ListAPIKeys":
	return verify(model.ProjectRBACResource_API_KEY, model.ProjectRBACPolicy_LIST)
	case "/grpc.service.webservice.WebService/AddApplication":

It reads the proto file and generates Go code in the following steps:

1. Make the method data from the service definition in the pkg/app/server/service/webservice/service.proto by using generateMethods ref
2. Finally, embed the value in the prepared template ref

Especially about 1, generateMethods do in the following steps:

1. Find the option with that name in the service ref
2. Convert option data to a string using v.String() and split the string with " " (two spaces)

   pipecd/tool/codegen/protoc-gen-auth/main.go

   Line 127 in 17a5007

   vs := strings.Split(v.String(), " ")

3. Parse these values ref

At timing 2, v.String() may be "resource:PIPED action:CREATE" (two spaces between elements) or "resource:PIPED action:CREATE (one space between elements)" .
This flaky behavior is the protobuf specification.

When you execute String() on a protobuf Message, MessageStringOf is called.
This is one of the example↓

// String returns a string representation of a message.
func (m *Message) String() string {
	return protoimpl.X.MessageStringOf(m)
}

ref: https://github.com/protocolbuffers/protobuf-go/blob/ec47fd138f9221b19a2afd6570b3c39ede9df3dc/types/dynamicpb/dynamic.go#L96-L99

MessageStringOf internally uses prototext.MarshalOptions.Format.

// MessageStringOf returns the message value as a string,
// which is the message serialized in the protobuf text format.
func (Export) MessageStringOf(m protoreflect.ProtoMessage) string {
	return prototext.MarshalOptions{Multiline: false}.Format(m)
}

ref: https://github.com/protocolbuffers/protobuf-go/blob/ec47fd138f9221b19a2afd6570b3c39ede9df3dc/internal/impl/api_export.go#L173-L177

According to the official doc the result of using prototext package is , flaky on purpose.
To avoid relying on something that is not clearly defined, it intentionally makes the value indefinite for undefined behavior.

According to these issues, the format for displaying protobuf as string is not clearly defined. So they introduce randomization of the result.

• encoding: provide canonical output format golang/protobuf#1121
• String and protojson produces non-deterministic whitespaces. golang/protobuf#1269

About the randomization:
Internally, a hash is created based on the executable binary, and the remainder of the hash divided by 2 is used to determine whether to insert additional spaces, and this value changes every time the program is compiled.
https://github.com/protocolbuffers/protobuf-go/blob/ec47fd138f9221b19a2afd6570b3c39ede9df3dc/internal/encoding/text/encode.go#L216-L234

Especially, it is archieved using detrand.Bool.
https://github.com/protocolbuffers/protobuf-go/blob/ec47fd138f9221b19a2afd6570b3c39ede9df3dc/internal/detrand/rand.go#L24-L69

Which issue(s) this PR fixes:

Fixes #5430

Does this PR introduce a user-facing change?:

• How are users affected by this change:
• Is this breaking change:
• How to migrate (if breaking change):

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.10%. Comparing base (511ee8f) to head (b9882f7).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5436   +/-   ##
=======================================
  Coverage   26.09%   26.10%           
=======================================
  Files         451      451           
  Lines       48824    48837   +13     
=======================================
+ Hits        12741    12748    +7     
- Misses      35065    35072    +7     
+ Partials     1018     1017    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[t-kikuc]

thank you!!1

[Warashi]

Thank you for a detailed description! I got it.