protoc-gen-validate icon indicating copy to clipboard operation
protoc-gen-validate copied to clipboard

Enum validation fails when enum defined in sibling package with same name

Open codyaray opened this issue 5 years ago • 6 comments

I just saw something that looks similar to #321, but in golang and with an incorrect package prefix rather than no prefix. This seems different enough to open a new issue, but I'm happy to merge if you'd like.

We include an enum from another package and set enum.defined_only constraint on it.

kafka.core.v1.MarketplacePartner partner = 2 [(validate.rules).enum.defined_only = true];

But the generated code looks like this:

	if _, ok := v1.MarketplacePartner_name[int32(m.GetPartner())]; m.maskHas(mask, "partner") && !ok {
		return MarketplaceEventValidationError{
			field:  "Partner",
			reason: "value must be one of the defined enum values",
		}
	}

Note that it says v1 instead of an import alias for kafka.core.v1. There's no import for kafka/core/v1 in the .pb.validate.go, and v1 is invalid since its not imported either.

More Details

This could have something to do with how we structure our proto files.

We use a proto monorepo which includes files with a layout like this

kafka
├── core
│   ├── go.mod
│   ├── go.sum
│   └── v1
│       ├── core.pb.go
│       ├── core.pb.validate.go
│       ├── core.proto
│       └── error.go
└── marketplace
       ├── go.mod
       ├── go.sum
       └── v1
           ├── marketplace.pb.go
           ├── marketplace.pb.validate.go
           └── marketplace.proto

Each proto package is nested in a v1, v2, etc directory and go_path is declared in the format like github.com/example/some/package/v1;v1.

The file kafka/marketplace/v1/marketplace.proto includes

syntax = "proto3";

import "kafka/core/v1/core.proto";
import "validate/validate.proto";

package kafka.marketplace.v1; 

option go_package = "github.com/confluentinc/cc-structs/kafka/marketplace/v1;v1";

message MarketplaceEvent {
  string id = 1; // uuid
  kafka.core.v1.MarketplacePartner partner = 2 [(validate.rules).enum.defined_only = true]; // GCP vs. AZURE vs. AWS ...
}

This references the file kafka/core/v1/core.proto, which includes

syntax = "proto3";

package kafka.core.v1;

import "validate/validate.proto";

option go_package = "github.com/confluentinc/cc-structs/kafka/core/v1;v1";

enum MarketplacePartner {
  UNKNOWN = 0;
  GCP = 1;
  AWS = 2;
  AZURE = 3;
}

The generated marketplace.pb.validate.go includes

// Code generated by protoc-gen-validate. DO NOT EDIT.
// source: kafka/marketplace/v1/marketplace.proto

package v1

import (
	"bytes"
	"errors"
	"fmt"
	"net"
	"net/mail"
	"net/url"
	"regexp"
	"strings"
	"time"
	"unicode/utf8"

	"github.com/golang/protobuf/ptypes"
	"github.com/iancoleman/strcase"
	"google.golang.org/genproto/protobuf/field_mask"
)

// ensure the imports are used
var (
	_ = bytes.MinRead
	_ = errors.New("")
	_ = fmt.Print
	_ = utf8.UTFMax
	_ = (*regexp.Regexp)(nil)
	_ = (*strings.Reader)(nil)
	_ = net.IPv4len
	_ = time.Duration(0)
	_ = (*url.URL)(nil)
	_ = (*mail.Address)(nil)
	_ = ptypes.DynamicAny{}
	_ = field_mask.FieldMask{}
	_ = strcase.ToSnake
)

// define the regex for a UUID once up-front
var _marketplace_uuidPattern = regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")

// Validate checks the field values on MarketplaceEvent with the rules defined
// in the proto definition for this message. If any rules are violated, an
// error is returned.
func (m *MarketplaceEvent) Validate() error {
	return m.ValidateWithMask(nil)
}

func (m *MarketplaceEvent) ValidateWithMask(mask *field_mask.FieldMask) error {
	if m == nil {
		return nil
	}

	// no validation rules for Id

	if _, ok := v1.MarketplacePartner_name[int32(m.GetPartner())]; m.maskHas(mask, "partner") && !ok {
		return MarketplaceEventValidationError{
			field:  "Partner",
			reason: "value must be one of the defined enum values",
		}
	}
       ...
}

Note the fieldmask/strcase imports come from https://github.com/envoyproxy/protoc-gen-validate/pull/366 but they shouldn't have anything to do with this issue.

We can see in the generated .pb.validate.go that we don't have any external enum imports at all.

codyaray avatar Aug 31 '20 23:08 codyaray

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 04 '20 02:10 stale[bot]

Probably would be good if this wasn't marked stale just because no one has had time to look at it at all yet. :)

codyaray avatar Oct 05 '20 18:10 codyaray

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 16 '20 03:11 stale[bot]

Hiya - I think this is still an issue

codyaray avatar Dec 15 '20 15:12 codyaray

I can confirm this is an issue -- no imports for enums defined in a different package.

hurrycane avatar Aug 05 '21 03:08 hurrycane

I can confirm it too https://github.com/envoyproxy/protoc-gen-validate/issues/585

mashail avatar Apr 27 '22 07:04 mashail