Integrate into protoc-gen-prost-validate #2
Comments
|
Hi! Thank you so much for tackling this :) First thing is I'm not able to compile the plugin binary (within Docker, that's a hard requirement for us). See this reproducer: docker buildx build . -o=. -f- <<'DOCKERFILE'
FROM rust:1.80-slim AS crate__protoc-gen-prost-validate
WORKDIR /app
ARG PROTOBUF_RELEASE_TAG=3.19.6
ADD --chmod=0664 --checksum=sha256:f51152d0926ccf18d89e2b4e4f31c4bf16ee5f94499d379029f80ddb8304bdd0 \
https://github.com/protocolbuffers/protobuf/releases/download/v$PROTOBUF_RELEASE_TAG/protoc-$PROTOBUF_RELEASE_TAG-linux-x86_64.zip protoc.zip
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && \
apt-get install --no-install-recommends -y \
unzip
RUN unzip protoc.zip -d protoc && \
mv ./protoc/bin/protoc /usr/bin/protoc && \
mv ./protoc/include/google /usr/local/include/google
#RUN ls -lha /usr/local/include/google /usr/local/include/google/protobuf && exit 42
RUN set -eux && cargo install --locked --git https://github.com/linka-cloud/protoc-gen-prost.git --branch protoc-gen-prost-validate protoc-gen-prost-validate
FROM scratch
COPY --from=crate__protoc-gen-prost-validate /usr/local/cargo/bin/protoc-gen-prost-validate /
DOCKERFILEErrors with: Note that this works when outside docker ( cargo install --locked --git https://github.com/linka-cloud/protoc-gen-prost.git --rev 51aa0aa3254126c439e78df93818d262b876417b protoc-gen-prost-validateI'm thinking https://github.com/linka-cloud/prost-validate/blob/30fcad4a122ad916823f7e15461ec0a2466943a7/prost-validate-types/build.rs isn't looking for globally-installed include files? |
|
Yup 52ae220 fixed it. |
|
Thanks @fenollp for your feedback. I cannot reproduce using: cat << EOF > Dockerfile
ARG TAG=1.74
FROM rust:\${TAG} AS builder
WORKDIR /src/protoc-gen-prost
RUN apt update && DEBIAN_FRONTEND=noninteractive apt install -y protobuf-compiler
RUN git clone https://github.com/linka-cloud/protoc-gen-prost -b protoc-gen-prost-validate .
RUN cargo build --release
FROM scratch
COPY --from=builder /src/protoc-gen-prost/target/release/protoc-gen-prost-validate /usr/local/bin/
EOF
docker buildx build .
# ...
docker buildx build --build-arg TAG=latest .
# ...But I get the issue with alpine based images: cat << EOF > Dockerfile
ARG TAG=1.74-alpine
FROM rust:${TAG} AS builder
WORKDIR /src/protoc-gen-prost
RUN apk add --no-cache protobuf git build-base
RUN git clone https://github.com/linka-cloud/protoc-gen-prost -b protoc-gen-prost-validate .
RUN cargo build --release
FROM scratch
COPY --from=builder /src/protoc-gen-prost/target/release/protoc-gen-prost-validate /usr/local/bin/
EOF
docker buildx build .
# Error: Custom { kind: Other, error: "protoc failed: google/protobuf/descriptor.proto: File not found.\ngoogle/protobuf/duration.proto: File not found.\ngoogle/protobuf/timestamp.proto: File not found.\nvalidate.proto:7:1: Import \"google/protobuf/descriptor.proto\" was not found or had errors.\nvalidate.proto:8:1: Import \"google/protobuf/duration.proto\" was not found or had errors.\nvalidate.proto:9:1: Import \"google/protobuf/timestamp.proto\" was not found or had errors.\nvalidate.proto:799:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:803:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:807:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:811:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:815:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:819:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:823:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:833:14: \"google.protobuf.Timestamp\" is not defined.\nvalidate.proto:837:14: \"google.protobuf.Timestamp\" is not defined.\nvalidate.proto:841:14: \"google.protobuf.Timestamp\" is not defined.\nvalidate.proto:845:14: \"google.protobuf.Timestamp\" is not defined.\nvalidate.proto:849:14: \"google.protobuf.Timestamp\" is not defined.\nvalidate.proto:862:14: \"google.protobuf.Duration\" is not defined.\nvalidate.proto:12:8: \"google.protobuf.MessageOptions\" is not defined.\nvalidate.proto: \"google.protobuf.MessageOptions\" is not defined.\nvalidate.proto:21:8: \"google.protobuf.OneofOptions\" is not defined.\nvalidate.proto:28:8: \"google.protobuf.FieldOptions\" is not defined.\n" } |
|
But this works: cat << EOF > Dockerfile
ARG TAG=1.74-alpine
FROM rust:${TAG} AS builder
WORKDIR /src/protoc-gen-prost
RUN apk add --no-cache protobuf protobuf-dev git build-base
RUN git clone https://github.com/linka-cloud/protoc-gen-prost -b protoc-gen-prost-validate .
RUN cargo build --release
FROM scratch
COPY --from=builder /src/protoc-gen-prost/target/release/protoc-gen-prost-validate /usr/local/bin/
EOF
docker buildx build .
# ...
docker buildx build --build-tab TAG=alpine . |
|
Thanks! Alright I got generation to work. However generated code fails to compile due to many of these: also on WKT and also on my types that just don't have
|
This is surprising. It seems that the field is evaluated as a message instead of a scalar. |
|
Ah I'm noticing that these trait issues on scalars are actually on WKT. Maybe I'm not setting the right options? Here's my buf config: version: v1
plugins:
- name: prost-validate
out: generated/rust/src/grpc
strategy: all
opt:
- compile_well_known_types
- enable_type_namesHere's a tiny reproducer for the other issue. I believe this simply has to do with nested messages (maybe in conjunction with google's types?). Note that there aren't any validation annotations in use in there syntax = "proto3";
import "google/api/annotations.proto";
import "google/protobuf/timestamp.proto";
service Api {
rpc Call(Req) returns (Rep) {}
}
message Req {
string id = 1;
}
message Rep {
message Nested {
string a = 1;
string b = 2;
string c = 3;
}
string f1 = 1;
string f2 = 2;
string f3 = 3;
string f4 = 4;
google.protobuf.Timestamp f5 = 5;
google.protobuf.Timestamp f6 = 6;
Nested f7 = 7;
string f8 = 8;
string f9 = 9;
} |
|
Thank you for the reproducer. Remove the |
|
Alright I'm disabling it. What do you mean "only supported with Here's another reproducer with nested messages and oneof: syntax = "proto3";
package company.product.v1;
import "google/api/annotations.proto";
service Api {
rpc Call(Req) returns (Rep) {}
}
message Req {
message NestedMsg {
string f1 = 1;
string f2 = 2;
}
oneof lookup_by {
NestedMsg o1 = 1;
string o2 = 2;
}
}
message Rep {
string f1 = 1;
} |
I mean that it does not work with There will indeed be validation issues with external messages because of the way in which I don't yet know how to solve this problem in Rust (if it's even possible). if v, ok := any(m.Message).(interface{ Validate() error }); ok {
if err := v.Validate(); err != nil {
return fmt.Errorf("....: %w", err)
}
}I suppose you need to mark the field as explicitly skipped, e.g: message Message {
External msg = 1 [(validate.field).message.skip = true];
}I'll take a look at your reproducer as soon as possible. |
|
Ok then, I'll continue skipping these options. I wasn't using the I see. Maybe this can be used as the Golang interface-casting test: https://github.com/nvzqz/impls#how-it-works Maybe the trick could be to impl the validation What's needed to impl handling of WKT? |
Already tried 😉, but it is unusable because it creates the same compilation issue.
This is exactly what prost-validate/prost-validate-build/src/lib.rs Lines 141 to 146 in 30fcad4 |
The Well-Known types are natively supported by Wrappers are generated in the same way as proto3 optionals are (it's transparent to the user: It really is a better user experience. But Support has been implemented. |
|
I've finally found a way to call the validate method only if it's implemented 😀. |
|
Here's another reproducer, this time for missing syntax = "proto3";
package company.product.v1;
import "validate/validate.proto";
message Msg {
oneof scope {
string a = 1 [(validate.rules).string.uuid = true];
string b = 2 [(validate.rules).string.uuid = true];
}
repeated string cs = 3 [(validate.rules).repeated = {
unique: true
items: { string: {uuid: true} }
}];
}// @generated by protoc-gen-prost-validate
impl ::prost_validate::Validator for Msg {
fn validate(&self) -> prost_validate::Result<()> {
let cs = &self.cs;
if ::prost_validate::VecExt::unique(cs).len() != cs.len() {
return Err(
::prost_validate::Error::new(
"company.product.v1.Msg.cs",
::prost_validate::errors::list::Error::Unique,
),
);
}
for (i, item) in cs.iter().enumerate() {
{
if let Err(_) = ::prost_validate::ValidateStringExt::validate_uuid(
&item,
) {
return Err(
::prost_validate::Error::new(
"company.product.v1.Msg.cs",
::prost_validate::errors::string::Error::Uuid,
),
);
}
Ok(())
}
.map_err(|e| ::prost_validate::Error::new(
format!("{}[{}]", "company.product.v1.Msg.cs", i),
::prost_validate::errors::list::Error::Item(Box::new(e)),
))?;
}
if let Some(ref scope) = self.scope {
::prost_validate::Validator::validate(scope)?;
}
Ok(())
}
} |
|
Thanks. I've got the problem: it's because sub-modules (created by oneof, enum and sub-messages) aren't handled in I'll fix it as soon as possible. |
|
|
|
I've found a solution. |
|
Thanks for your great work and reactivity! syntax = "proto3";
import "validate/validate.proto";
message A {
message B {
enum X {
e1 = 0;
e2 = 1;
e3 = 2;
}
X x = 4 [(validate.rules).enum.defined_only = true];
}
}Note: it doesn't reproduce if enum isn't nested. |
|
You're welcome ! 😀 Can you give me a link or paste the contents of the generated validation code and your input message, please? I was a bit too quick to push a fix on enum's sub-modules, so I'm surprised it even compiles in this case... |
|
Well this crash comes from the execution of your plugin (version neoeinstein/protoc-gen-prost@e26dc79 ) through buf (version |
Sorry, I read a bit too quickly. I'll check tomorrow. |
|
😂 yes, that's exactly the problem I was thinking. |
|
Done. |
|
It all compiles! Thanks! Now I'll take time in the coming days to actually test the generated code. |
|
Tests adapted from the official test suite are implemented here for |
Hi! I saw your comment today on the protoc
This issue asks to integrate with https://github.com/neoeinstein/protoc-gen-prost/tree/main/protoc-gen-prost-validate
I believe first some work is required to support prost generated files instead of straight up file descriptor sets.
Note that ideally this protobuf plugin should be usable through
protocorbufand generate files inOUT_DIR, as is the case with the other protoc plugins in @neoeinstein's repo.I'd like to help! :)
The text was updated successfully, but these errors were encountered: