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.

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

Migrate proto generation to buf #1139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 29 additions & 5 deletions .golangci.yaml → .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
run:
concurrency: 4
timeout: 10m
allow-parallel-runners: true

linters:
disable-all: true
Copy link
Member

Choose a reason for hiding this comment

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

Why do we first disable all linters? Isn't it possible to invert it? Enable all and disable the not needed linters?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @lukasfrank, other projects (looked at prometheus and some grafana projects) use the same pattern to only disable unwanted linters. Golang CI Lint also adds linters with new updates which would be removed, as such you have the overhead to also review those changes as they come in. With an inverse you will eventually see failed golangcilint runs and can decide if this a valid issue or if the linter (or its check) should be disabled.

As per golangcilint documentation both enable-all and disable-all fields are false by default to achieve both patterns :)

enable:
- revive
- ginkgolinter
- copyloopvar
- errcheck
- goconst
- gofmt
- goimports
- gosimple
- govet
- ineffassign
- ginkgolinter
- misspell
- goimports
- importas
- nakedret
- staticcheck
- typecheck
- unconvert
- unparam
- unused

severity:
Expand Down Expand Up @@ -38,5 +48,19 @@ linters-settings:
alias: ${1}client

issues:
# exclude copylocks issues in fake servers and unit tests
exclude-files:
- iri/testing/volume/fake.go
- iri/testing/bucket/fake.go
- iri/testing/machine/fake.go
- poollet/machinepoollet/controllers/machine_controller_test.go
# TODO: fis copylocks issues in those files
- poollet/volumepoollet/vcm/generic.go
- poollet/bucketpoollet/bcm/generic.go
# don't skip warning about doc comments
# don't exclude the default set of lint
exclude-use-default: false
# restore some of the defaults
# (fill in the rest as needed)
exclude: # Exclude stutter issues (for now)
- "exported: type name will be used as (.+) by other packages, and that stutters; consider calling this (.+)"
7 changes: 6 additions & 1 deletion .reuse/dep5
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ Source: https://github.com/ironcore-dev/ironcore
Files:
.github/*
.gitignore
.golangci.yaml
.golangci.yml
buf.gen.yaml
buf.yaml
CODEOWNERS
Dockerfile
Makefile
Expand All @@ -20,6 +22,9 @@ Files:
go.mod
go.sum
hack/*
iri/apis/bucket/v1alpha1/api_grpc.pb.go
iri/apis/machine/v1alpha1/api_grpc.pb.go
iri/apis/volume/v1alpha1/api_grpc.pb.go
iri/apis/bucket/v1alpha1/api.pb.go
iri/apis/bucket/v1alpha1/api.proto
iri/apis/event/v1alpha1/api.pb.go
Expand Down
22 changes: 10 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,8 @@ generate: vgopath models-schema openapi-gen
./hack/update-codegen.sh

.PHONY: proto
proto: goimports vgopath protoc-gen-gogo
VGOPATH=$(VGOPATH) \
PROTOC_GEN_GOGO=$(PROTOC_GEN_GOGO) \
./hack/update-proto.sh
proto: goimports vgopath buf
$(BUF) generate --template buf.gen.yaml
$(GOIMPORTS) -w ./iri

.PHONY: fmt
Expand All @@ -101,7 +99,7 @@ fmt: goimports ## Run goimports against code.

.PHONY: vet
vet: ## Run go vet against code.
go vet ./...
go vet -copylocks=false ./...

.PHONY: lint
lint: golangci-lint ## Run golangci-lint on the code.
Expand Down Expand Up @@ -145,7 +143,7 @@ clean-docs: ## Remove all local mkdocs Docker images (cleanup).
docker container prune --force --filter "label=project=ironcore_documentation"

.PHONY: test
test: manifests generate fmt vet test-only ## Run tests.
test: manifests generate proto fmt vet test-only ## Run tests.

.PHONY: test-only
test-only: envtest ## Run *only* the tests - no generation, linting etc.
Expand Down Expand Up @@ -352,7 +350,7 @@ OPENAPI_GEN ?= $(LOCALBIN)/openapi-gen
VGOPATH ?= $(LOCALBIN)/vgopath
GEN_CRD_API_REFERENCE_DOCS ?= $(LOCALBIN)/gen-crd-api-reference-docs
ADDLICENSE ?= $(LOCALBIN)/addlicense
PROTOC_GEN_GOGO ?= $(LOCALBIN)/protoc-gen-gogo
BUF ?= $(LOCALBIN)/buf
MODELS_SCHEMA ?= $(LOCALBIN)/models-schema
GOIMPORTS ?= $(LOCALBIN)/goimports
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
Expand All @@ -363,10 +361,10 @@ VGOPATH_VERSION ?= v0.1.3
CONTROLLER_TOOLS_VERSION ?= v0.15.0
GEN_CRD_API_REFERENCE_DOCS_VERSION ?= v0.3.0
ADDLICENSE_VERSION ?= v1.1.1
PROTOC_GEN_GOGO_VERSION ?= v1.3.2
GOIMPORTS_VERSION ?= v0.26.0
GOLANGCI_LINT_VERSION ?= v1.61.0
OPENAPI_EXTRACTOR_VERSION ?= v0.1.4
BUF_VERSION ?= v1.45.0

KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
.PHONY: kustomize
Expand Down Expand Up @@ -421,10 +419,10 @@ addlicense: $(ADDLICENSE) ## Download addlicense locally if necessary.
$(ADDLICENSE): $(LOCALBIN)
test -s $(LOCALBIN)/addlicense || GOBIN=$(LOCALBIN) go install github.com/google/addlicense@$(ADDLICENSE_VERSION)

.PHONY: protoc-gen-gogo
protoc-gen-gogo: $(PROTOC_GEN_GOGO) ## Download protoc-gen-gogo locally if necessary.
$(PROTOC_GEN_GOGO): $(LOCALBIN)
test -s $(LOCALBIN)/protoc-gen-gogo || GOBIN=$(LOCALBIN) go install github.com/gogo/protobuf/protoc-gen-gogo@$(PROTOC_GEN_GOGO_VERSION)
.PHONY: buf
buf: $(BUF) ## Download buf locally if necessary.
$(BUF): $(LOCALBIN)
test -s $(LOCALBIN)/buf || GOBIN=$(LOCALBIN) go install github.com/bufbuild/buf/cmd/buf@$(BUF_VERSION)

.PHONY: models-schema
models-schema: $(MODELS_SCHEMA) ## Install models-schema locally if necessary.
Expand Down
10 changes: 3 additions & 7 deletions broker/bucketbroker/server/bucketclass_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (s *Server) filterIronCoreBucketClasses(
return filtered
}

func (s *Server) convertIronCoreBucketClass(bucketClass *storagev1alpha1.BucketClass) (*iri.BucketClass, error) {
func (s *Server) convertIronCoreBucketClass(bucketClass *storagev1alpha1.BucketClass) *iri.BucketClass {
tps := bucketClass.Capabilities.TPS()
iops := bucketClass.Capabilities.IOPS()

Expand All @@ -70,7 +70,7 @@ func (s *Server) convertIronCoreBucketClass(bucketClass *storagev1alpha1.BucketC
Tps: tps.Value(),
Iops: iops.Value(),
},
}, nil
}
}

func (s *Server) ListBucketClasses(ctx context.Context, req *iri.ListBucketClassesRequest) (*iri.ListBucketClassesResponse, error) {
Expand Down Expand Up @@ -99,11 +99,7 @@ func (s *Server) ListBucketClasses(ctx context.Context, req *iri.ListBucketClass
availableIronCoreBucketClasses := s.filterIronCoreBucketClasses(availableIronCoreBucketClassNames, ironcoreBucketClassList.Items)
bucketClasses := make([]*iri.BucketClass, 0, len(availableIronCoreBucketClasses))
for _, ironcoreBucketClass := range availableIronCoreBucketClasses {
bucketClass, err := s.convertIronCoreBucketClass(&ironcoreBucketClass)
if err != nil {
return nil, fmt.Errorf("error converting ironcore bucket class %s: %w", ironcoreBucketClass.Name, err)
}

bucketClass := s.convertIronCoreBucketClass(&ironcoreBucketClass)
bucketClasses = append(bucketClasses, bucketClass)
}

Expand Down
1 change: 1 addition & 0 deletions broker/bucketbroker/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func init() {

type Server struct {
client client.Client
iri.UnimplementedBucketRuntimeServer

namespace string
bucketPoolName string
Expand Down
4 changes: 2 additions & 2 deletions broker/machinebroker/cmd/machinebroker/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func Run(ctx context.Context, opts Options) error {

g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
return runServer(ctx, setupLog, log, srv)
return runServer(ctx, setupLog, srv)
})
g.Go(func() error {
return runGRPCServer(ctx, setupLog, log, srv, opts)
Expand All @@ -133,7 +133,7 @@ func Run(ctx context.Context, opts Options) error {
return g.Wait()
}

func runServer(ctx context.Context, setupLog, log logr.Logger, srv *server.Server) error {
func runServer(ctx context.Context, setupLog logr.Logger, srv *server.Server) error {
setupLog.V(1).Info("Starting server loops")
if err := srv.Start(ctx); err != nil {
return fmt.Errorf("error starting server loops: %w", err)
Expand Down
10 changes: 3 additions & 7 deletions broker/machinebroker/server/machine_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (s *Server) prepareIronCoreMachinePower(power iri.Power) (computev1alpha1.P
}
}

func (s *Server) prepareIronCoreMachineLabels(machine *iri.Machine) (map[string]string, error) {
func (s *Server) prepareIronCoreMachineLabels(machine *iri.Machine) map[string]string {
labels := make(map[string]string)

for downwardAPILabelName, defaultLabelName := range s.brokerDownwardAPILabels {
Expand All @@ -62,7 +62,7 @@ func (s *Server) prepareIronCoreMachineLabels(machine *iri.Machine) (map[string]
}
}

return labels, nil
return labels
}

func (s *Server) prepareIronCoreMachineAnnotations(machine *iri.Machine) (map[string]string, error) {
Expand Down Expand Up @@ -113,11 +113,7 @@ func (s *Server) getIronCoreMachineConfig(machine *iri.Machine) (*IronCoreMachin
ironcoreVolumeCfgs[i] = ironcoreVolumeCfg
}

labels, err := s.prepareIronCoreMachineLabels(machine)
if err != nil {
return nil, fmt.Errorf("error preparing ironcore machine labels: %w", err)
}

labels := s.prepareIronCoreMachineLabels(machine)
annotations, err := s.prepareIronCoreMachineAnnotations(machine)
if err != nil {
return nil, fmt.Errorf("error preparing ironcore machine annotations: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion broker/machinebroker/server/machine_volume_attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (s *Server) getIronCoreVolumeConfig(volume *iri.Volume) (*IronCoreVolumeCon
case volume.EmptyDisk != nil:
var sizeLimit *resource.Quantity
if sizeBytes := volume.EmptyDisk.SizeBytes; sizeBytes > 0 {
sizeLimit = resource.NewQuantity(int64(sizeBytes), resource.DecimalSI)
sizeLimit = resource.NewQuantity(sizeBytes, resource.DecimalSI)
}
emptyDisk = &IronCoreVolumeEmptyDiskConfig{
SizeLimit: sizeLimit,
Expand Down
2 changes: 2 additions & 0 deletions broker/machinebroker/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type BrokerLabel struct {
}

type Server struct {
iri.UnimplementedMachineRuntimeServer

baseURL *url.URL

brokerDownwardAPILabels map[string]string
Expand Down
10 changes: 3 additions & 7 deletions broker/machinebroker/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *Server) filterIronCoreMachineClasses(
return filtered
}

func (s *Server) convertIronCoreMachineClassStatus(machineClass *computev1alpha1.MachineClass, quantity *resource.Quantity) (*iri.MachineClassStatus, error) {
func (s *Server) convertIronCoreMachineClassStatus(machineClass *computev1alpha1.MachineClass, quantity *resource.Quantity) *iri.MachineClassStatus {
cpu := machineClass.Capabilities.CPU()
memory := machineClass.Capabilities.Memory()

Expand All @@ -91,7 +91,7 @@ func (s *Server) convertIronCoreMachineClassStatus(machineClass *computev1alpha1
},
},
Quantity: quantity.Value(),
}, nil
}
}

func (s *Server) Status(ctx context.Context, req *iri.StatusRequest) (*iri.StatusResponse, error) {
Expand Down Expand Up @@ -129,11 +129,7 @@ func (s *Server) Status(ctx context.Context, req *iri.StatusRequest) (*iri.Statu
continue
}

machineClass, err := s.convertIronCoreMachineClassStatus(&ironcoreMachineClass, quantity)
if err != nil {
return nil, fmt.Errorf("error converting ironcore machine class %s: %w", ironcoreMachineClass.Name, err)
}

machineClass := s.convertIronCoreMachineClassStatus(&ironcoreMachineClass, quantity)
machineClassStatus = append(machineClassStatus, machineClass)
}

Expand Down
2 changes: 2 additions & 0 deletions broker/volumebroker/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ func init() {
}

type Server struct {
iri.UnimplementedVolumeRuntimeServer

client client.Client
idGen idgen.IDGen

Expand Down
10 changes: 3 additions & 7 deletions broker/volumebroker/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (s *Server) filterIronCoreVolumeClasses(
return filtered
}

func (s *Server) convertIronCoreVolumeClassStatus(volumeClass *storagev1alpha1.VolumeClass, quantity *resource.Quantity) (*iri.VolumeClassStatus, error) {
func (s *Server) convertIronCoreVolumeClassStatus(volumeClass *storagev1alpha1.VolumeClass, quantity *resource.Quantity) *iri.VolumeClassStatus {
tps := volumeClass.Capabilities.TPS()
iops := volumeClass.Capabilities.IOPS()

Expand All @@ -90,7 +90,7 @@ func (s *Server) convertIronCoreVolumeClassStatus(volumeClass *storagev1alpha1.V
},
},
Quantity: quantity.Value(),
}, nil
}
}

func (s *Server) Status(ctx context.Context, req *iri.StatusRequest) (*iri.StatusResponse, error) {
Expand Down Expand Up @@ -128,11 +128,7 @@ func (s *Server) Status(ctx context.Context, req *iri.StatusRequest) (*iri.Statu
continue
}

volumeClass, err := s.convertIronCoreVolumeClassStatus(&ironcoreVolumeClass, quantity)
if err != nil {
return nil, fmt.Errorf("error converting ironcore volume class %s: %w", ironcoreVolumeClass.Name, err)
}

volumeClass := s.convertIronCoreVolumeClassStatus(&ironcoreVolumeClass, quantity)
volumeClassStatus = append(volumeClassStatus, volumeClass)
}

Expand Down
2 changes: 1 addition & 1 deletion broker/volumebroker/server/volume_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (s *Server) getIronCoreVolumeConfig(_ context.Context, volume *iri.Volume)
VolumePoolSelector: s.volumePoolSelector,
VolumePoolRef: volumePoolRef,
Resources: corev1alpha1.ResourceList{
corev1alpha1.ResourceStorage: *resource.NewQuantity(int64(volume.Spec.Resources.StorageBytes), resource.DecimalSI),
corev1alpha1.ResourceStorage: *resource.NewQuantity(volume.Spec.Resources.StorageBytes, resource.DecimalSI),
},
Image: volume.Spec.Image,
ImagePullSecretRef: nil, // TODO: Fill if necessary
Expand Down
2 changes: 1 addition & 1 deletion broker/volumebroker/server/volume_expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s *Server) ExpandVolume(ctx context.Context, req *iri.ExpandVolumeRequest)

log.V(1).Info("Expanding volume")
if err := s.setIronCoreVolumeResources(ctx, ironcoreVolume.Volume, corev1alpha1.ResourceList{
corev1alpha1.ResourceStorage: *resource.NewQuantity(int64(req.Resources.StorageBytes), resource.DecimalSI),
corev1alpha1.ResourceStorage: *resource.NewQuantity(req.Resources.StorageBytes, resource.DecimalSI),
}); err != nil {
return nil, fmt.Errorf("failed to expand volume: %w", err)
}
Expand Down
13 changes: 13 additions & 0 deletions buf.gen.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: v2
plugins:
# The protoc-gen-go stubs are required for grpc-go
- remote: buf.build/protocolbuffers/go
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we are calling an external API by using a remote plugin.
We might want to get rid of it.

➜  ironcore git:(enh/proto-generation) ✗ buf generate
Failure: too many requests

Please see https://buf.build/docs/bsr/rate-limits for details about BSR rate limiting.

out: iri/apis
# You almost always want to set this option with protoc-gen-go
opt: paths=source_relative
- remote: buf.build/grpc/go
# Make sure to generate your grpc-go code to the same
# directory as protoc-gen-go
out: iri/apis
# You almost always want to set this option with protoc-gen-go-grpc
opt: paths=source_relative
9 changes: 9 additions & 0 deletions buf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: v2
modules:
- path: iri/apis
lint:
use:
- STANDARD
breaking:
use:
- FILE
7 changes: 4 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
module github.com/ironcore-dev/ironcore

go 1.23.0
go 1.23.2

require (
github.com/afritzler/protoequal v0.1.1
github.com/bits-and-blooms/bitset v1.14.3
github.com/blang/semver/v4 v4.0.0
github.com/go-chi/chi/v5 v5.1.0
github.com/go-logr/logr v1.4.2
github.com/gogo/protobuf v1.3.2
github.com/google/go-cmp v0.6.0
github.com/ironcore-dev/controller-utils v0.9.4
github.com/onsi/ginkgo/v2 v2.20.2
Expand All @@ -19,6 +19,7 @@ require (
golang.org/x/sync v0.8.0
golang.org/x/sys v0.26.0
google.golang.org/grpc v1.67.1
google.golang.org/protobuf v1.35.1
k8s.io/api v0.30.4
k8s.io/apimachinery v0.30.4
k8s.io/apiserver v0.30.4
Expand Down Expand Up @@ -57,6 +58,7 @@ require (
github.com/go-openapi/jsonreference v0.20.2 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/cel-go v0.17.8 // indirect
Expand Down Expand Up @@ -113,7 +115,6 @@ require (
google.golang.org/genproto v0.0.0-20240227224415-6ceb2ff114de // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect
google.golang.org/protobuf v1.34.2 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
Expand Down
Loading