-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update CRD and fix minor issues #164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
../../../../root/go/pkg/mod/github.com/redpanda-data/helm-charts@v0.0.0-20240624182334-f539d8f02f09/charts/redpanda/helpers.go:311:8: cannot infer T (/root/go/pkg/mod/k8s.io/utils@v0.0.0-20240310230437-4693a0247e57/ptr/ptr.go:56:12) (typecheck)
--
| sc := ptr.Deref(values.Statefulset.PodSecurityContext, values.Statefulset.SecurityContext)
| ^
| ../../../../root/go/pkg/mod/github.com/redpanda-data/helm-charts@v0.0.0-20240624182334-f539d8f02f09/charts/redpanda/helpers.go:325:8: cannot infer T (/root/go/pkg/mod/k8s.io/utils@v0.0.0-20240310230437-4693a0247e57/ptr/ptr.go:56:12) (typecheck)
| sc := ptr.Deref(values.Statefulset.PodSecurityContext, values.Statefulset.SecurityContext)
| ^
| ../../../../root/go/pkg/mod/github.com/redpanda-data/helm-charts@v0.0.0-20240624182334-f539d8f02f09/charts/redpanda/service.loadbalancer.go:86:8: cannot infer T (/root/go/pkg/mod/k8s.io/utils@v0.0.0-20240310230437-4693a0247e57/ptr/ptr.go:56:12) (typecheck)
| if !ptr.Deref(listener.Enabled, values.External.Enabled) {
| ^
| ../../../../root/go/pkg/mod/github.com/redpanda-data/helm-charts@v0.0.0-20240624182334-f539d8f02f09/pkg/gotohelm/helmette/sprig.go:313:17: cannot range over n (variable of type int) (typecheck)
| for i := range n {
I feel like we should disable or update typecheck
linter. It might be a problem that helm chart is using 1.22.4 https://github.com/redpanda-data/helm-charts/blob/981683cc95b7013673d3ea28c753cb048232f8ce/go.mod#L3 go version. redpanda-operator on the other hand is using 1.21
redpanda-operator/src/go/k8s/go.mod
Line 3 in ff0c0c1
go 1.21 |
Prior to this commit golangci-lint was installed by task and run in buildkite, installed by an action and run in github actions, and there existed two `.golangci.yml` files. This scattered configuration was frustrating to deal with and resulted in linting rules being applied inconsistently (aside from the golangci's own inconsistency). This commit collapses the configurations down to a single file in `src/go/k8s/.golangci.yml`, updates the configurations to fix some deprecations, removes the github action for linting in favor of builtkite, installed golangci-lint via nix, and fixes pre-existing errors discovered by fixing the configuration spaghetti.
This commit updates `RedpandaClusterSpec` to support the `TrustStore` option that was added to the helm chart in redpanda-data/helm-charts@f539d8f. Additionally, it adds a regression test to find divergences between the CRD and helm chart. The majority of this test case is currently disabled due to many such divergences. Finally, this commit also contains a handful of small fixes that were found in the above test case: - Added `omitempty` to `Auth.SASL`, `RedpandaMemory.{Memory,ReserveMemory}`. - Deprecated `ListenerTLS.SecretRef` as it's not a real value. - Deprecated `UsageStats.Organization` as it's been removed from the helm chart. - Removed unused types `Limits`, `Requests`, `TopologySpreadConstraintsItems`. - Corrected `TopologySpreadConstraint` to be a slice. (See note) Technically, changing the type of `TopologySpreadConstraint` is a breaking change. However, specifying it would have caused deployments to immediately break. Therefore it's reasonably safe to assume this change will not affect any users. Changing the casing of `FullNameOverride` is being deferred as it is possible to specify it without breaking deployments and changing the casing could result in breakages as that would be seen as a field addition and removal from the API perspective.
176d56e
to
81ff038
Compare
I fixed up out golangci-lint configuration in this commit e2560d7. Linting should be much more consistent now though you'll need to rely on nix develop and task to run it until we deal with the structure of the repo in more detail. |
This commit updates
RedpandaClusterSpec
to support theTrustStore
option that was added to the helm chart inredpanda-data/helm-charts@f539d8f.
Additionally, it adds a regression test to find divergences between the CRD and helm chart. The majority of this test case is currently disabled due to many such divergences.
Finally, this commit also contains a handful of small fixes that were found in the above test case:
omitempty
toAuth.SASL
,RedpandaMemory.{Memory,ReserveMemory}
.ListenerTLS.SecretRef
as it's not a real value.UsageStats.Organization
as it's been removed from the helm chart.Limits
,Requests
,TopologySpreadConstraintsItems
.TopologySpreadConstraint
to be a slice. (See note)Technically, changing the type of
TopologySpreadConstraint
is a breaking change. However, specifying it would have caused deployments to immediately break. Therefore it's reasonably safe to assume this change will not affect any users.Changing the casing of
FullNameOverride
is being deferred as it is possible to specify it without breaking deployments and changing the casing could result in breakages as that would be seen as a field addition and removal from the API perspective.