Skip to content

add protobuf definition and generated Golang code which is buildable with TinyGo - #7

Closed
sanposhiho wants to merge 2 commits into
kubernetes-sigs:mainfrom
sanposhiho:kubernetes-api
Closed

add protobuf definition and generated Golang code which is buildable with TinyGo#7
sanposhiho wants to merge 2 commits into
kubernetes-sigs:mainfrom
sanposhiho:kubernetes-api

Conversation

@sanposhiho

Copy link
Copy Markdown
Member

What type of PR is this?

/kind feature

What this PR does / why we need it:

add protobuf definition + generated Golang protobuf code which is buildable with TinyGo.
We cannot use the proto definitions in kubernetes/api because it depends on many stuff which aren't buildable with TinyGo.
(Hopefully, it'll be buildable in the future version of TinyGo.)

The protobuf definition is generated from https://github.com/nytimes/openapi2proto with the patch. And use google.golang.org/protobuf/cmd/protoc-gen-go for Golang code generation.

As you see in ./kubernetes/proto/go.mod, it only depends on google.golang.org/protobuf, which is buildable with TinyGo.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 21, 2023
@k8s-ci-robot
k8s-ci-robot requested a review from kerthcet May 21, 2023 05:16
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 21, 2023
@sanposhiho

Copy link
Copy Markdown
Member Author

/cc @codefromthecrypt

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

@sanposhiho: GitHub didn't allow me to request PR reviews from the following users: codefromthecrypt.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @codefromthecrypt

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@codefromthecrypt

Copy link
Copy Markdown
Contributor

Maybe you can add a test that shows hot to do the following flow? This will help make sure it still works. The test may need to be in the plugin module, in order to avoid leaking v1 deps to the guest.

  • Convert from v1.Pod to api.IoK8SApiCoreV1Pod
  • Marshal api.IoK8SApiCoreV1Pod to proto
  • UnMarshal api.IoK8SApiCoreV1Pod from proto
  • Convert back to v1.Pod (optional)

@codefromthecrypt codefromthecrypt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feel free to merge and/or push commits to my branch.

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codefromthecrypt, sanposhiho

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codefromthecrypt

Copy link
Copy Markdown
Contributor

I made notes on #6 and not sure how much of #7 (comment) is needed. Basically, first thing is to make sure we can build tinygo wasm which reads it. For some reason my laptop doesn't respond when trying to build, both 0.27 and dev version of tinygo.

@codefromthecrypt

Copy link
Copy Markdown
Contributor

In #6 I added code to generate the MarshallVT functions

@sanposhiho

Copy link
Copy Markdown
Member Author

It's included in #6.

/close

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

@sanposhiho: Closed this PR.

Details

In response to this:

It's included in #6.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2023
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants