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

refactor(evpn-gw): updates to support buf lint and generation #411

Merged
merged 4 commits into from
Dec 12, 2023
Merged

refactor(evpn-gw): updates to support buf lint and generation #411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d0654a
refactor(evpn-gw): updates to support buf lint and generation
sandersms Dec 5, 2023
93660f5
fix(evpn-gw): enable buf generate in full make
sandersms Dec 7, 2023
cf3267d
Merge branch 'opiproject:main' into evpn-automate
sandersms Dec 11, 2023
e0ec268
fix(evpn-gw): add comment lint checks for evpn-gw
sandersms Dec 11, 2023
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
2 changes: 1 addition & 1 deletion buf.work.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ directories:
# - storage
- network/opinetcommon
# - network/cloud
# - network/evpn-gw
- network/evpn-gw
# - network/k8s
# - network/telco
24 changes: 12 additions & 12 deletions network/evpn-gw/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
# Copyright (c) 2022 Dell Inc, or its subsidiaries.
# Copyright (C) 2023 Nordix Foundation.

all:
all: buflint doc
Copy link
Contributor

Choose a reason for hiding this comment

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

do we intentionally not run bufgen target? Without bufgen, the CI job does not actually check that we provided all generated artifacts

opi-api/.github/workflows/network.yml

Lines 42 to 48 in b990048

- name: Build protobufs
run: make
working-directory: network
- name: Check uncomitted auto generated protobufs
run: git diff --exit-code
working-directory: network

I missed that point on the review of opinetcommon as well...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will add that into the steps currently. Eventually the generated files will be placed into a separate repository and autogenerated; but, for now adding the bufgen in the make should take care of the operation.


doc:
rm -rf ./google
rm -rf ./v1alpha1/{autogen.md,gen,google}
mkdir -p ./v1alpha1/gen/{go,cpp,python,java}
rm -rf ./v1alpha1/{autogen.md}
Copy link
Contributor

Choose a reason for hiding this comment

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

{} are not needed since we have only one option autogen.md
Also, since it is a single file, we can just use rm -f

mkdir -p ./v1alpha1

# protoc doesn't include annotation and http googleapis, so we have to get them here
curl -kL https://github.com/googleapis/googleapis/archive/master.tar.gz | tar --strip=1 -zxvf - googleapis-master/google/api

docker run --user=$$(id -u):$$(id -g) --rm -v "${PWD}":/defs -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v ${PWD}/google/api:/opt/include/google/api namely/protoc-all:1.51_2 -i /common -i /opinetcommon --lint -d v1alpha1 -l go -o ./v1alpha1/gen/go/ --go-source-relative
docker run --user=$$(id -u):$$(id -g) --rm -v "${PWD}":/defs -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v ${PWD}/google/api:/opt/include/google/api namely/protoc-all:1.51_2 -i /common -i /opinetcommon --lint -d v1alpha1 -l cpp -o ./v1alpha1/gen/cpp/ --go-source-relative
docker run --user=$$(id -u):$$(id -g) --rm -v "${PWD}":/defs -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v ${PWD}/google/api:/opt/include/google/api namely/protoc-all:1.51_2 -i /common -i /opinetcommon --lint -d v1alpha1 -l python -o ./v1alpha1/gen/python/ --go-source-relative
docker run --user=$$(id -u):$$(id -g) --rm -v "${PWD}":/defs -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v ${PWD}/google/api:/opt/include/google/api namely/protoc-all:1.51_2 -i /common -i /opinetcommon --lint -d v1alpha1 -l java -o ./v1alpha1/gen/java/ --go-source-relative
docker run --user=$$(id -u):$$(id -g) --rm --entrypoint=sh -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v "${PWD}"/v1alpha1/:/out -w /out -v "${PWD}":/protos pseudomuto/protoc-gen-doc:1.5.1 -c "protoc -I /common -I /opinetcommon -I /protos --doc_out=/out --doc_opt=markdown,autogen.md /protos/*.proto /common/*.proto"

rm -rf "${PWD}"/google

mv google "${PWD}"/v1alpha1/
buflint:
docker run --rm -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinnetcommon -v "${PWD}":/out -w /out bufbuild/buf lint

docker run --user=$$(id -u):$$(id -g) --rm --entrypoint=sh -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v "${PWD}"/v1alpha1/:/out -w /out -v "${PWD}"/v1alpha1:/protos pseudomuto/protoc-gen-doc:1.5.1 -c "protoc -I /common -I /opinetcommon -I /protos --doc_out=/out --doc_opt=markdown,autogen.md /protos/*.proto /common/*.proto"
docker run --user=$$(id -u):$$(id -g) --rm --entrypoint=sh -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v "${PWD}"/v1alpha1/:/out -w /out ghcr.io/docker-multiarch/google-api-linter:1.59.2 -c "api-linter -I /common -I /opinetcommon /out/*.proto --output-format summary"
docker run --user=$$(id -u):$$(id -g) --rm --entrypoint=sh -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinetcommon -v "${PWD}"/v1alpha1/:/out -w /out ghcr.io/docker-multiarch/google-api-linter:1.59.2 -c "api-linter -I /common -I /opinetcommon /out/[!oc]*.proto --output-format github --set-exit-status"
rm -rf "${PWD}"/v1alpha1/google
bufgen:
docker run --rm -v "${PWD}/../../common/v1":/common -v "${PWD}/../opinetcommon":/opinnetcommon -v "${PWD}/../..":/base -v "${PWD}":/out -w /out msandersdell/bufbuild-go-gen:1.1.0 generate --template /base/buf.gen.yaml -o v1alpha1
13 changes: 9 additions & 4 deletions network/evpn-gw/buf.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@ deps:
- remote: buf.build
owner: googleapis
repository: googleapis
commit: 28151c0d0a1641bf938a7672c500e01d
digest: shake256:49215edf8ef57f7863004539deff8834cfb2195113f0b890dd1f67815d9353e28e668019165b9d872395871eeafcbab3ccfdb2b5f11734d3cca95be9e8d139de
commit: b30c5775bfb3485d9da2e87b26590ac9
digest: shake256:9d0caaf056949a0e1c883b9849d8a2fa66e22f18a2a48f867d1a8c700aa22abee50ad3ef0d8171637457cadc43c584998bdf3adac55da0f9e4614c72686b057d
- remote: buf.build
owner: grpc-ecosystem
repository: grpc-gateway
commit: f460f71081c14a80b66cc72526e0b322
digest: shake256:122def85e91fc3ef4ab351680060b8f70e9d09a7ae6d1412aeb2bddfeee5c4d3fc8819da33fef56192cec0a817ac0c3e6d49bb2acf02eb5c9e9131739a60ddfc
commit: 3f42134f4c564983838425bc43c7a65f
digest: shake256:3d11d4c0fe5e05fda0131afefbce233940e27f0c31c5d4e385686aea58ccd30f72053f61af432fa83f1fc11cda57f5f18ca3da26a29064f73c5a0d076bba8d92
- remote: buf.build
owner: opiproject
repository: opinetcommon
commit: 9779e55c83de44b3a14925b8195cf7e7
digest: shake256:20d43132a22c5d3341919f5167669f137b79b23e807a5c34a3168216a28e07a114f3f7a94b73925f824befd5862ac354d5ad45a3136761ca429f041286657524
8 changes: 8 additions & 0 deletions network/evpn-gw/buf.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,11 @@ name: buf.build/opiproject/evpn-gw
deps:
- buf.build/googleapis/googleapis
- buf.build/grpc-ecosystem/grpc-gateway
- buf.build/opiproject/opinetcommon
lint:
Copy link
Contributor

Choose a reason for hiding this comment

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

let's also include COMMENTS linter rules here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to latest addition.

except:
- PACKAGE_DIRECTORY_MATCH
# Don't check standard name as that causes google aip issues
- RPC_RESPONSE_STANDARD_NAME
# Allow same name used in request/response type for multiple RPCs
- RPC_REQUEST_RESPONSE_UNIQUE
6 changes: 3 additions & 3 deletions ...k/evpn-gw/v1alpha1/l2_xpu_infra_mgr.proto → network/evpn-gw/l2_xpu_infra_mgr.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,9 @@ enum BPOperStatus {
// BridgePortType reflects the different types of a Bridge Port
enum BridgePortType {
// "unknown" bridge port type
UNKNOWN = 0;
BRIDGE_PORT_TYPE_UNSPECIFIED = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason that we have changed the ENUM here ? Does this follows the googles AIPs for ENUMs ?

Copy link
Contributor Author

@sandersms sandersms Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes - this follows the AIPs for ENUMs as part of the checking. It also requires the addition of the "BRIDGE_PORT_TYPE_" for all of the ENUMs to be unique as a check.

// "access" bridge port type
ACCESS = 1;
BRIDGE_PORT_TYPE_ACCESS = 1;
// "trunk" bridge port type
TRUNK = 2;
BRIDGE_PORT_TYPE_TRUNK = 2;
}
0 ...k/evpn-gw/v1alpha1/l3_xpu_infra_mgr.proto → network/evpn-gw/l3_xpu_infra_mgr.proto
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions network/evpn-gw/v1alpha1/autogen.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,9 @@ BridgePortType reflects the different types of a Bridge Port

| Name | Number | Description |
| ---- | ------ | ----------- |
| UNKNOWN | 0 | "unknown" bridge port type |
| ACCESS | 1 | "access" bridge port type |
| TRUNK | 2 | "trunk" bridge port type |
| BRIDGE_PORT_TYPE_UNSPECIFIED | 0 | "unknown" bridge port type |
| BRIDGE_PORT_TYPE_ACCESS | 1 | "access" bridge port type |
| BRIDGE_PORT_TYPE_TRUNK | 2 | "trunk" bridge port type |



Expand Down
Loading
Loading