Skip to content

Commit

Permalink
Fix specifying of Kubernetes version from build scripts
Browse files Browse the repository at this point in the history
This bug came about because of three issues that this change
corrects:
- The CI scripts run on a pull request did not test building
  Helm. This means that a failure to set a variable using LDFLAGS
  had no opportunity to be caught.
- helm#8608 provided a means to match the k8s version used in linting
  and chartutil with the version of the package we pull in. With
  one problem. It attempts to set a const as if it were a string.
  This is ignored and everyone missed it.
- helm#10325 moved those constants to vars so it could be set. This
  looked good and passed tests but missed that you can't set an
  int as if it were a string. See first bullet.

This change fixes this by moved the internal representation to
be a string. These are internal variables not exposed in the public
API which makes this change non-breaking to the API.

Closes helm#10367

Signed-off-by: Matt Farina <[email protected]>
  • Loading branch information
mattfarina committed Nov 18, 2021
1 parent f866942 commit 7838fb7
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jobs:
- run:
name: test
command: make test-coverage
- run:
name: test build
command: make
- deploy:
name: deploy
command: .circleci/deploy.sh
Expand Down
12 changes: 7 additions & 5 deletions pkg/chartutil/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,20 @@ import (
)

var (
k8sVersionMajor = 1
k8sVersionMinor = 20
// The Kubernetes version can be set by LDFLAGS. In order to do that the value
// must be a string.
k8sVersionMajor = "1"
k8sVersionMinor = "20"

// DefaultVersionSet is the default version set, which includes only Core V1 ("v1").
DefaultVersionSet = allKnownVersions()

// DefaultCapabilities is the default set of capabilities.
DefaultCapabilities = &Capabilities{
KubeVersion: KubeVersion{
Version: fmt.Sprintf("v%d.%d.0", k8sVersionMajor, k8sVersionMinor),
Major: strconv.Itoa(k8sVersionMajor),
Minor: strconv.Itoa(k8sVersionMinor),
Version: fmt.Sprintf("v%s.%s.0", k8sVersionMajor, k8sVersionMinor),
Major: k8sVersionMajor,
Minor: k8sVersionMinor,
},
APIVersions: DefaultVersionSet,
HelmVersion: helmversion.Get(),
Expand Down
19 changes: 15 additions & 4 deletions pkg/lint/rules/deprecations.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rules // import "helm.sh/helm/v3/pkg/lint/rules"

import (
"fmt"
"strconv"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -27,9 +28,10 @@ import (

var (
// This should be set in the Makefile based on the version of client-go being imported.
// These constants will be overwritten with LDFLAGS
k8sVersionMajor = 1
k8sVersionMinor = 20
// These constants will be overwritten with LDFLAGS. The version components must be
// strings in order for LDFLAGS to set them.
k8sVersionMajor = "1"
k8sVersionMinor = "20"
)

// deprecatedAPIError indicates than an API is deprecated in Kubernetes
Expand Down Expand Up @@ -60,7 +62,16 @@ func validateNoDeprecations(resource *K8sYamlStruct) error {
}
return err
}
if !deprecation.IsDeprecated(runtimeObject, k8sVersionMajor, k8sVersionMinor) {
maj, err := strconv.Atoi(k8sVersionMajor)
if err != nil {
return err
}
min, err := strconv.Atoi(k8sVersionMinor)
if err != nil {
return err
}

if !deprecation.IsDeprecated(runtimeObject, maj, min) {
return nil
}
gvk := fmt.Sprintf("%s %s", resource.APIVersion, resource.Kind)
Expand Down

0 comments on commit 7838fb7

Please sign in to comment.