diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index a490134a..05ed971a 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -3,6 +3,8 @@ on: pull_request: branches: - 'main' + paths: + - "!README.md" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -58,10 +60,13 @@ jobs: - name: Run all unit tests run: make test - - name: Check test coverage - run: | - go install github.com/vladopajic/go-test-coverage/v2@latest - go-test-coverage --config=./.testcoverage.yml + - name: check test coverage + uses: vladopajic/go-test-coverage@v2 + with: + config: ./.testcoverage.yml + + - name: Trigger Coverage update + uses: ./coverage-badge.yaml - name: Generate code coverage artifacts uses: actions/upload-artifact@v4 diff --git a/.github/workflows/coverage-badge.yaml b/.github/workflows/coverage-badge.yaml new file mode 100644 index 00000000..887f86da --- /dev/null +++ b/.github/workflows/coverage-badge.yaml @@ -0,0 +1,55 @@ +name: Generate code coverage badge + +on: + workflow_dispatch: # Here for Testing + workflow_call: + +permissions: + contents: write + +jobs: + test: + runs-on: ubuntu-latest + name: Update coverage badge + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of your personal access token. + fetch-depth: 0 # otherwise, there would be errors pushing refs to the destination repository. + + - name: Setup go + uses: actions/setup-go@v4 + with: + go-version-file: 'go.mod' + + - name: Run Test + run: | + go test -v ./... -covermode=count -coverprofile=coverage.out + go tool cover -func=coverage.out -o=coverage.out + + - name: Go Coverage Badge # Pass the `coverage.out` output to this action + uses: tj-actions/coverage-badge-go@v2 + with: + filename: coverage.out + + - name: Verify Changed files + uses: tj-actions/verify-changed-files@v16 + id: verify-changed-files + with: + files: README.md + + - name: Commit changes + if: steps.verify-changed-files.outputs.files_changed == 'true' + run: | + git config --local user.email "action@github.com" + git config --local user.name "GitHub Action" + git add README.md + git commit -m "docs: Updated coverage badge." + + - name: Push changes + if: steps.verify-changed-files.outputs.files_changed == 'true' + uses: ad-m/github-push-action@master + with: + github_token: ${{ github.token }} + branch: ${{ github.head_ref }} diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 1f0284f2..25af08a1 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -52,7 +52,6 @@ jobs: include: "Makefile" regex: true - - name: Install Helm Docs uses: envoy/install-helm-docs@v1.0.0 with: @@ -94,7 +93,6 @@ jobs: id: github_release uses: mikepenz/release-changelog-builder-action@v5 - - name: Create Release PR uses: devops-infra/action-pull-request@v0.5.5 with: diff --git a/README.md b/README.md index e1bc58d1..f3cef586 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,10 @@ # version-checker +![GitHub Release](https://img.shields.io/github/v/release/jetstack/version-checker) +[![Go Report Card](https://goreportcard.com/badge/github.com/jetstack/version-checker)](https://goreportcard.com/report/github.com/jetstack/version-checker) +[![Tests](https://github.com/jetstack/version-checker/actions/workflows/build-test.yaml/badge.svg)](https://github.com/jetstack/version-checker/actions/workflows/build-test.yaml?query=branch%3Amain) +![GitHub go.mod Go version](https://img.shields.io/github/go-mod/go-version/jetstack/version-checker) + version-checker is a Kubernetes utility for observing the current versions of images running in the cluster, as well as the latest available upstream. These checks get exposed as Prometheus metrics to be viewed on a dashboard, or _soft_ diff --git a/cmd/app/app.go b/cmd/app/app.go index 9cd6ed70..647e2bc4 100644 --- a/cmd/app/app.go +++ b/cmd/app/app.go @@ -27,7 +27,7 @@ func NewCommand(ctx context.Context) *cobra.Command { Use: "version-checker", Short: helpOutput, Long: helpOutput, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { opts.complete() logLevel, err := logrus.ParseLevel(opts.LogLevel) diff --git a/cmd/app/options.go b/cmd/app/options.go index 358a04af..4c82ccdf 100644 --- a/cmd/app/options.go +++ b/cmd/app/options.go @@ -60,7 +60,7 @@ var ( selfhostedInsecureReg = regexp.MustCompile("^VERSION_CHECKER_SELFHOSTED_INSECURE_(.*)") ) -// Options is a struct to hold options for the version-checker +// Options is a struct to hold options for the version-checker. type Options struct { MetricsServingAddress string DefaultTestAll bool @@ -88,7 +88,7 @@ func (o *Options) addFlags(cmd *cobra.Command) { return nil }) - cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { + cmd.SetHelpFunc(func(cmd *cobra.Command, _ []string) { fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n"+usageFmt, cmd.Long, cmd.UseLine()) cliflag.PrintSections(cmd.OutOrStdout(), nfs, 0) }) @@ -329,55 +329,53 @@ func (o *Options) assignSelfhosted(envs []string) { } } - for _, env := range envs { - pair := strings.SplitN(env, "=", 2) - if len(pair) != 2 || len(pair[1]) == 0 { - continue - } - - if matches := selfhostedHostReg.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + regexActions := map[*regexp.Regexp]func(matches []string, value string){ + selfhostedHostReg: func(matches []string, value string) { initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].Host = pair[1] - continue - } - - if matches := selfhostedUsernameReg.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + o.Client.Selfhosted[matches[1]].Host = value + }, + selfhostedUsernameReg: func(matches []string, value string) { initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].Username = pair[1] - continue - } - - if matches := selfhostedPasswordReg.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + o.Client.Selfhosted[matches[1]].Username = value + }, + selfhostedPasswordReg: func(matches []string, value string) { initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].Password = pair[1] - continue - } - - if matches := selfhostedTokenPath.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + o.Client.Selfhosted[matches[1]].Password = value + }, + selfhostedTokenPath: func(matches []string, value string) { initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].TokenPath = pair[1] - continue - } - - if matches := selfhostedTokenReg.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + o.Client.Selfhosted[matches[1]].TokenPath = value + }, + selfhostedTokenReg: func(matches []string, value string) { initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].Bearer = pair[1] - continue - } - - if matches := selfhostedInsecureReg.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { + o.Client.Selfhosted[matches[1]].Bearer = value + }, + selfhostedInsecureReg: func(matches []string, value string) { initOptions(matches[1]) - val, err := strconv.ParseBool(pair[1]) - if err == nil { + if val, err := strconv.ParseBool(value); err == nil { o.Client.Selfhosted[matches[1]].Insecure = val } + }, + selfhostedCAPath: func(matches []string, value string) { + initOptions(matches[1]) + o.Client.Selfhosted[matches[1]].CAPath = value + }, + } + + for _, env := range envs { + pair := strings.SplitN(env, "=", 2) + if len(pair) != 2 || len(pair[1]) == 0 { continue } - if matches := selfhostedCAPath.FindStringSubmatch(strings.ToUpper(pair[0])); len(matches) == 2 { - initOptions(matches[1]) - o.Client.Selfhosted[matches[1]].CAPath = pair[1] - continue + key := strings.ToUpper(pair[0]) + value := pair[1] + + for regex, action := range regexActions { + if matches := regex.FindStringSubmatch(key); len(matches) == 2 { + action(matches, value) + break + } } } diff --git a/cmd/app/options_test.go b/cmd/app/options_test.go index 3f8b60a3..86796b00 100644 --- a/cmd/app/options_test.go +++ b/cmd/app/options_test.go @@ -73,7 +73,7 @@ func TestComplete(t *testing.T) { Token: "quay-token", }, Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", @@ -141,21 +141,21 @@ func TestComplete(t *testing.T) { Token: "quay-token", }, Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", Bearer: "my-token", Insecure: true, }, - "BAR": &selfhosted.Options{ + "BAR": { Host: "bar.docker.joshvanl.com", Username: "bar.joshvanl", Password: "bar-password", Bearer: "my-bar-token", Insecure: false, }, - "BUZZ": &selfhosted.Options{ + "BUZZ": { Host: "buzz.docker.jetstack.io", Username: "buzz.davidcollom", Password: "buzz-password", @@ -208,7 +208,7 @@ func TestAssignSelfhosted(t *testing.T) { }, expOptions: client.Options{ Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", @@ -228,13 +228,13 @@ func TestAssignSelfhosted(t *testing.T) { }, expOptions: client.Options{ Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", Bearer: "my-token", }, - "BAR": &selfhosted.Options{ + "BAR": { Host: "hello.world.com", Bearer: "my-bar-token", }, @@ -253,14 +253,14 @@ func TestAssignSelfhosted(t *testing.T) { }, expOptions: client.Options{ Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", Bearer: "my-token", TokenPath: "/artifactory/api/security/token", }, - "BAR": &selfhosted.Options{ + "BAR": { Host: "hello.world.com", Bearer: "my-bar-token", }, @@ -281,17 +281,17 @@ func TestAssignSelfhosted(t *testing.T) { }, expOptions: client.Options{ Selfhosted: map[string]*selfhosted.Options{ - "FOO": &selfhosted.Options{ + "FOO": { Host: "docker.joshvanl.com", Username: "joshvanl", Password: "password", Bearer: "my-token", }, - "BAR": &selfhosted.Options{ + "BAR": { Host: "hello.world.com", Bearer: "my-bar-token", }, - "JOSHVANL": &selfhosted.Options{ + "JOSHVANL": { Host: "joshvanl.com", }, }, @@ -301,7 +301,6 @@ func TestAssignSelfhosted(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { - o := new(Options) o.assignSelfhosted(test.envs) diff --git a/deploy/charts/version-checker/README.md b/deploy/charts/version-checker/README.md index 53a64161..1f54dadd 100644 --- a/deploy/charts/version-checker/README.md +++ b/deploy/charts/version-checker/README.md @@ -56,6 +56,12 @@ A Helm chart for version-checker | replicaCount | int | `1` | Replica Count for version-checker | | resources | object | `{}` | Setup version-checkers resource requests/limits | | securityContext | object | `{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"runAsUser":65534,"seccompProfile":{"type":"RuntimeDefault"}}` | Set container-level security context | +| securityContext.allowPrivilegeEscalation | bool | `false` | Prevent the container from PrivilegeEscalation | +| securityContext.capabilities | object | `{"drop":["ALL"]}` | Ensure that we run with the capabilities we explicitly need to run | +| securityContext.readOnlyRootFilesystem | bool | `true` | Readonly Filesystem | +| securityContext.runAsNonRoot | bool | `true` | Ensure we don't run as root | +| securityContext.runAsUser | int | `65534` | Specify UID to run under | +| securityContext.seccompProfile | object | `{"type":"RuntimeDefault"}` | SeccomProfile to use | | selfhosted | []{name: "", host: "", username:"", password:"", token:""}] | `[]` | Setup a number of SelfHosted Repositories and their credentials | | service.annotations | object | `{}` | Additional annotations to add to the service | | service.labels | object | `{}` | Additional labels to add to the service | diff --git a/deploy/charts/version-checker/templates/_helpers.tpl b/deploy/charts/version-checker/templates/_helpers.tpl index 6a311a8e..1ca664e5 100644 --- a/deploy/charts/version-checker/templates/_helpers.tpl +++ b/deploy/charts/version-checker/templates/_helpers.tpl @@ -32,4 +32,4 @@ Common selector {{- define "version-checker.selector" -}} app.kubernetes.io/name: {{ include "version-checker.name" . }} app.kubernetes.io/instance: {{ .Release.Name }} -{{- end -}} \ No newline at end of file +{{- end -}} diff --git a/deploy/charts/version-checker/values.yaml b/deploy/charts/version-checker/values.yaml index 5d6f39a6..cf0756a9 100644 --- a/deploy/charts/version-checker/values.yaml +++ b/deploy/charts/version-checker/values.yaml @@ -127,13 +127,19 @@ resources: # -- Set container-level security context securityContext: + # -- Prevent the container from PrivilegeEscalation allowPrivilegeEscalation: false + # -- Ensure that we run with the capabilities we explicitly need to run capabilities: drop: - ALL + # -- Readonly Filesystem readOnlyRootFilesystem: true + # -- Ensure we don't run as root runAsNonRoot: true + # -- Specify UID to run under runAsUser: 65534 + # -- SeccomProfile to use seccompProfile: type: RuntimeDefault diff --git a/deploy/yaml/deploy.yaml b/deploy/yaml/deploy.yaml index f5d72838..677e93d4 100644 --- a/deploy/yaml/deploy.yaml +++ b/deploy/yaml/deploy.yaml @@ -1,3 +1,4 @@ +--- apiVersion: v1 kind: Namespace metadata: @@ -49,33 +50,33 @@ spec: spec: serviceAccountName: version-checker containers: - - image: quay.io/jetstack/version-checker:v0.8.1 - imagePullPolicy: Always - ports: - - containerPort: 8080 - name: version-checker - command: ["version-checker"] - livenessProbe: - httpGet: - path: /readyz - port: 8080 - initialDelaySeconds: 3 - periodSeconds: 3 - readinessProbe: - httpGet: - path: /readyz - port: 8080 - initialDelaySeconds: 3 - periodSeconds: 3 + - image: quay.io/jetstack/version-checker:v0.8.1 + imagePullPolicy: Always + ports: + - containerPort: 8080 + name: version-checker + command: ["version-checker"] + livenessProbe: + httpGet: + path: /readyz + port: 8080 + initialDelaySeconds: 3 + periodSeconds: 3 + readinessProbe: + httpGet: + path: /readyz + port: 8080 + initialDelaySeconds: 3 + periodSeconds: 3 --- kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 metadata: name: version-checker rules: -- apiGroups: [""] - resources: ["pods"] - verbs: ["get", "watch", "list"] + - apiGroups: [""] + resources: ["pods"] + verbs: ["get", "watch", "list"] --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 @@ -86,6 +87,6 @@ roleRef: kind: ClusterRole name: version-checker subjects: -- kind: ServiceAccount - name: version-checker - namespace: version-checker + - kind: ServiceAccount + name: version-checker + namespace: version-checker diff --git a/deploy/yaml/kustomization.yaml b/deploy/yaml/kustomization.yaml index 97476f0d..b6084601 100644 --- a/deploy/yaml/kustomization.yaml +++ b/deploy/yaml/kustomization.yaml @@ -1,4 +1,5 @@ +--- apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization resources: -- deploy.yaml + - deploy.yaml diff --git a/go.mod b/go.mod index 34bfc659..63daf45c 100644 --- a/go.mod +++ b/go.mod @@ -26,8 +26,8 @@ require ( ) require ( - github.com/aws/aws-sdk-go-v2/config v1.27.30 - github.com/aws/aws-sdk-go-v2/credentials v1.17.29 + github.com/aws/aws-sdk-go-v2/config v1.27.31 + github.com/aws/aws-sdk-go-v2/credentials v1.17.30 github.com/aws/aws-sdk-go-v2/service/ecr v1.32.2 github.com/gofri/go-github-ratelimit v1.1.0 github.com/google/go-containerregistry v0.20.2 @@ -119,7 +119,7 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect - k8s.io/kube-openapi v0.0.0-20240816214639-573285566f34 // indirect + k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.17.3 // indirect sigs.k8s.io/kustomize/kyaml v0.17.2 // indirect diff --git a/go.sum b/go.sum index 79f65140..5377d1ad 100644 --- a/go.sum +++ b/go.sum @@ -18,10 +18,10 @@ github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUM github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= github.com/aws/aws-sdk-go-v2 v1.30.4 h1:frhcagrVNrzmT95RJImMHgabt99vkXGslubDaDagTk8= github.com/aws/aws-sdk-go-v2 v1.30.4/go.mod h1:CT+ZPWXbYrci8chcARI3OmI/qgd+f6WtuLOoaIA8PR0= -github.com/aws/aws-sdk-go-v2/config v1.27.30 h1:AQF3/+rOgeJBQP3iI4vojlPib5X6eeOYoa/af7OxAYg= -github.com/aws/aws-sdk-go-v2/config v1.27.30/go.mod h1:yxqvuubha9Vw8stEgNiStO+yZpP68Wm9hLmcm+R/Qk4= -github.com/aws/aws-sdk-go-v2/credentials v1.17.29 h1:CwGsupsXIlAFYuDVHv1nnK0wnxO0wZ/g1L8DSK/xiIw= -github.com/aws/aws-sdk-go-v2/credentials v1.17.29/go.mod h1:BPJ/yXV92ZVq6G8uYvbU0gSl8q94UB63nMT5ctNO38g= +github.com/aws/aws-sdk-go-v2/config v1.27.31 h1:kxBoRsjhT3pq0cKthgj6RU6bXTm/2SgdoUMyrVw0rAI= +github.com/aws/aws-sdk-go-v2/config v1.27.31/go.mod h1:z04nZdSWFPaDwK3DdJOG2r+scLQzMYuJeW0CujEm9FM= +github.com/aws/aws-sdk-go-v2/credentials v1.17.30 h1:aau/oYFtibVovr2rDt8FHlU17BTicFEMAi29V1U+L5Q= +github.com/aws/aws-sdk-go-v2/credentials v1.17.30/go.mod h1:BPJ/yXV92ZVq6G8uYvbU0gSl8q94UB63nMT5ctNO38g= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.12 h1:yjwoSyDZF8Jth+mUk5lSPJCkMC0lMy6FaCD51jm6ayE= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.12/go.mod h1:fuR57fAgMk7ot3WcNQfb6rSEn+SUffl7ri+aa8uKysI= github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.16 h1:TNyt/+X43KJ9IJJMjKfa3bNTiZbUP7DeCxfbTROESwY= @@ -172,10 +172,10 @@ github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/ github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= -github.com/onsi/ginkgo/v2 v2.20.0 h1:PE84V2mHqoT1sglvHc8ZdQtPcwmvvt29WLEEO3xmdZw= -github.com/onsi/ginkgo/v2 v2.20.0/go.mod h1:lG9ey2Z29hR41WMVthyJBGUBcBhGOtoPF2VFMvBXFCI= -github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k= -github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY= +github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA= +github.com/onsi/ginkgo/v2 v2.19.0/go.mod h1:rlwLi9PilAFJ8jCg9UE1QP6VBpd6/xj3SRC0d6TU0To= +github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= +github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= @@ -240,8 +240,6 @@ golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58 golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/crypto v0.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw= golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54= -golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= -golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= @@ -340,8 +338,8 @@ k8s.io/component-base v0.31.0 h1:/KIzGM5EvPNQcYgwq5NwoQBaOlVFrghoVGr8lG6vNRs= k8s.io/component-base v0.31.0/go.mod h1:TYVuzI1QmN4L5ItVdMSXKvH7/DtvIuas5/mm8YT3rTo= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= -k8s.io/kube-openapi v0.0.0-20240816214639-573285566f34 h1:/amS69DLm09mtbFtN3+LyygSFohnYGMseF8iv+2zulg= -k8s.io/kube-openapi v0.0.0-20240816214639-573285566f34/go.mod h1:G0W3eI9gG219NHRq3h5uQaRBl4pj4ZpwzRP5ti8y770= +k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 h1:GKE9U8BH16uynoxQii0auTjmmmuZ3O0LFMN6S0lPPhI= +k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2/go.mod h1:coRQXBK9NxO98XUv3ZD6AK3xzHCxV6+b7lrquKwaKzA= k8s.io/utils v0.0.0-20240821151609-f90d01438635 h1:2wThSvJoW/Ncn9TmQEYXRnevZXi2duqHWf5OX9S3zjI= k8s.io/utils v0.0.0-20240821151609-f90d01438635/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= diff --git a/pkg/api/types.go b/pkg/api/types.go index b7a6cbb0..59bb44c4 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -26,7 +26,7 @@ const ( // UseMetaDataAnnotationKey is defined as a tag containing anything after the // patch digit. - // e.g. v1.0.1-gke.3 v1.0.1-alpha.0, v1.2.3.4 + // e.g. v1.0.1-gke.3 v1.0.1-alpha.0, v1.2.3.4... UseMetaDataAnnotationKey = "use-metadata.version-checker.io" // PinMajorAnnotationKey will pin the major version to check. diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index eda828d2..cfcca64b 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -29,13 +29,13 @@ type cacheItem struct { i interface{} } -// Handler is an interface for implementations of the cache fetch +// Handler is an interface for implementations of the cache fetch. type Handler interface { // Fetch should fetch an item by the given index and options Fetch(ctx context.Context, index string, opts *api.Options) (interface{}, error) } -// New returns a new generic Cache +// New returns a new generic Cache. func New(log *logrus.Entry, timeout time.Duration, handler Handler) *Cache { return &Cache{ log: log.WithField("cache", "handler"), @@ -99,7 +99,6 @@ func (c *Cache) StartGarbageCollector(refreshRate time.Duration) { now := time.Now() for index, item := range c.store { if item.timestamp.Add(c.timeout).Before(now) { - log.Debugf("removing stale cache item: %q", index) delete(c.store, index) } diff --git a/pkg/client/acr/acr.go b/pkg/client/acr/acr.go index 630f8989..3419ca7d 100644 --- a/pkg/client/acr/acr.go +++ b/pkg/client/acr/acr.go @@ -41,11 +41,11 @@ type Options struct { RefreshToken string } -type ACRAccessTokenResponse struct { +type AccessTokenResponse struct { AccessToken string `json:"access_token"` } -type ACRManifestResponse struct { +type ManifestResponse struct { Manifests []struct { Digest string `json:"digest"` CreatedTime time.Time `json:"createdTime"` @@ -84,8 +84,9 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag if err != nil { return nil, err } + defer resp.Body.Close() - var manifestResp ACRManifestResponse + var manifestResp ManifestResponse if err := json.NewDecoder(resp.Body).Decode(&manifestResp); err != nil { return nil, fmt.Errorf("%s: failed to decode manifest response: %s", host, err) @@ -177,7 +178,7 @@ func (c *Client) getACRClient(ctx context.Context, host string) (*acrClient, err return client, nil } -func (c *Client) getBasicAuthClient(host string) (*acrClient, error) { +func (c *Client) getBasicAuthClient(_ string) (*acrClient, error) { client := autorest.NewClientWithUserAgent(userAgent) client.Authorizer = autorest.NewBasicAuthorizer(c.Username, c.Password) @@ -216,8 +217,9 @@ func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrCli return nil, fmt.Errorf("%s: failed to request access token: %s", host, err) } + defer resp.Body.Close() - var respToken ACRAccessTokenResponse + var respToken AccessTokenResponse if err := json.NewDecoder(resp.Body).Decode(&respToken); err != nil { return nil, fmt.Errorf("%s: failed to decode access token response: %s", host, err) @@ -242,7 +244,6 @@ func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrCli } func getTokenExpiration(tokenString string) (time.Time, error) { - token, err := jwt.Parse(tokenString, nil, jwt.WithoutClaimsValidation()) if err != nil { return time.Time{}, err diff --git a/pkg/client/client.go b/pkg/client/client.go index 90011bed..8b54fa9b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -109,7 +109,7 @@ func (c *Client) Tags(ctx context.Context, imageURL string) ([]api.ImageTag, err } // fromImageURL will return the appropriate registry client for a given -// image URL, and the host + path to search +// image URL, and the host + path to search. func (c *Client) fromImageURL(imageURL string) (ImageClient, string, string) { var host, path string diff --git a/pkg/client/docker/docker.go b/pkg/client/docker/docker.go index 27b2dbb6..78e70d2c 100644 --- a/pkg/client/docker/docker.go +++ b/pkg/client/docker/docker.go @@ -140,6 +140,7 @@ func (c *Client) doRequest(ctx context.Context, url string) (*TagResponse, error if err != nil { return nil, fmt.Errorf("failed to get docker image: %s", err) } + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) if err != nil { @@ -173,6 +174,7 @@ func basicAuthSetup(ctx context.Context, client *http.Client, opts Options) (str if err != nil { return "", err } + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/pkg/client/ecr/ecr.go b/pkg/client/ecr/ecr.go index 3e70f4e4..a7b0305a 100644 --- a/pkg/client/ecr/ecr.go +++ b/pkg/client/ecr/ecr.go @@ -64,7 +64,6 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag var tags []api.ImageTag for _, img := range images.ImageDetails { - // Continue early if no tags available if len(img.ImageTags) == 0 { tags = append(tags, api.ImageTag{ diff --git a/pkg/client/fallback/fallback.go b/pkg/client/fallback/fallback.go index 02fe52df..f41e7998 100644 --- a/pkg/client/fallback/fallback.go +++ b/pkg/client/fallback/fallback.go @@ -42,7 +42,7 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag return c.OCI.Tags(ctx, host, repo, image) } -func (c *Client) IsHost(host string) bool { +func (c *Client) IsHost(_ string) bool { return true } diff --git a/pkg/client/gcr/gcr.go b/pkg/client/gcr/gcr.go index b539a94b..05ec261e 100644 --- a/pkg/client/gcr/gcr.go +++ b/pkg/client/gcr/gcr.go @@ -48,27 +48,19 @@ func (c *Client) Name() string { } func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.ImageTag, error) { - if repo != "" { - image = fmt.Sprintf("%s/%s", repo, image) - } - + image = c.constructImageName(repo, image) url := fmt.Sprintf(lookupURL, host, image) - req, err := http.NewRequest(http.MethodGet, url, nil) + req, err := c.buildRequest(ctx, url) if err != nil { return nil, err } - if len(c.Token) > 0 { - req.SetBasicAuth("oauth2accesstoken", c.Token) - } - - req = req.WithContext(ctx) - resp, err := c.Do(req) if err != nil { - return nil, fmt.Errorf("failed to get docker image: %s", err) + return nil, fmt.Errorf("failed to get docker image: %w", err) } + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) if err != nil { @@ -80,15 +72,37 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag return nil, err } + return c.extractImageTags(response) +} + +func (c *Client) constructImageName(repo, image string) string { + if repo != "" { + return fmt.Sprintf("%s/%s", repo, image) + } + return image +} + +func (c *Client) buildRequest(ctx context.Context, url string) (*http.Request, error) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + if len(c.Token) > 0 { + req.SetBasicAuth("oauth2accesstoken", c.Token) + } + + return req.WithContext(ctx), nil +} + +func (c *Client) extractImageTags(response Response) ([]api.ImageTag, error) { var tags []api.ImageTag for sha, manifestItem := range response.Manifest { - miliTimestamp, err := strconv.ParseInt(manifestItem.TimeCreated, 10, 64) + timestamp, err := c.convertTimestamp(manifestItem.TimeCreated) if err != nil { - return nil, fmt.Errorf("failed to convert timestamp string: %s", err) + return nil, fmt.Errorf("failed to convert timestamp string: %w", err) } - timestamp := time.Unix(0, miliTimestamp*int64(1000000)) - // If no tag, add without and continue early. if len(manifestItem.Tag) == 0 { tags = append(tags, api.ImageTag{SHA: sha, Timestamp: timestamp}) @@ -99,6 +113,13 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag tags = append(tags, api.ImageTag{Tag: tag, SHA: sha, Timestamp: timestamp}) } } - return tags, nil } + +func (c *Client) convertTimestamp(timeCreated string) (time.Time, error) { + miliTimestamp, err := strconv.ParseInt(timeCreated, 10, 64) + if err != nil { + return time.Time{}, err + } + return time.Unix(0, miliTimestamp*int64(1000000)), nil +} diff --git a/pkg/client/ghcr/ghcr.go b/pkg/client/ghcr/ghcr.go index 03e7f6ba..9133750d 100644 --- a/pkg/client/ghcr/ghcr.go +++ b/pkg/client/ghcr/ghcr.go @@ -44,75 +44,88 @@ func (c *Client) Name() string { return "ghcr" } -func (c *Client) Tags(ctx context.Context, host, owner, repo string) ([]api.ImageTag, error) { - // Choose the correct list packages function based on whether the owner - // is a user or an organization +func (c *Client) Tags(ctx context.Context, _, owner, repo string) ([]api.ImageTag, error) { + // Determine the correct function to get all versions based on the owner type + getAllVersions, repo, err := c.determineGetAllVersionsFunc(ctx, owner, repo) + if err != nil { + return nil, err + } + + opts := c.buildPackageListOptions() + + var tags []api.ImageTag + for { + versions, resp, err := getAllVersions(ctx, owner, "container", repo, opts) + if err != nil { + return nil, fmt.Errorf("getting versions: %w", err) + } + + tags = append(tags, c.extractImageTags(versions)...) + + if resp.NextPage == 0 { + break + } + + opts.ListOptions.Page = resp.NextPage + } + + return tags, nil +} + +func (c *Client) determineGetAllVersionsFunc(ctx context.Context, owner, repo string) (func(ctx context.Context, owner, pkgType, repo string, opts *github.PackageListOptions) ([]*github.PackageVersion, *github.Response, error), string, error) { getAllVersions := c.client.Organizations.PackageGetAllVersions ownerType, err := c.ownerType(ctx, owner) if err != nil { - return nil, fmt.Errorf("fetching owner type: %w", err) + return nil, "", fmt.Errorf("fetching owner type: %w", err) } if ownerType == "user" { getAllVersions = c.client.Users.PackageGetAllVersions - // The User implementation doesn't path escape this for you: - // - https://github.com/google/go-github/blob/v58.0.0/github/users_packages.go#L136 - // - https://github.com/google/go-github/blob/v58.0.0/github/orgs_packages.go#L105 repo = url.PathEscape(repo) } + return getAllVersions, repo, nil +} - opts := &github.PackageListOptions{ +func (c *Client) buildPackageListOptions() *github.PackageListOptions { + return &github.PackageListOptions{ PackageType: github.String("container"), State: github.String("active"), ListOptions: github.ListOptions{ PerPage: 100, }, } +} +func (c *Client) extractImageTags(versions []*github.PackageVersion) []api.ImageTag { var tags []api.ImageTag + for _, ver := range versions { + if len(ver.Metadata.Container.Tags) == 0 { + continue + } - for { - versions, resp, err := getAllVersions(ctx, owner, "container", repo, opts) - if err != nil { - return nil, fmt.Errorf("getting versions: %w", err) + sha := "" + if strings.HasPrefix(*ver.Name, "sha") { + sha = *ver.Name } - for _, ver := range versions { - if len(ver.Metadata.Container.Tags) == 0 { + for _, tag := range ver.Metadata.Container.Tags { + if c.shouldSkipTag(tag) { continue } - sha := "" - if strings.HasPrefix(*ver.Name, "sha") { - sha = *ver.Name - } - - for _, tag := range ver.Metadata.Container.Tags { - // Exclude attestations, signatures and sboms - if strings.HasSuffix(tag, ".att") { - continue - } - if strings.HasSuffix(tag, ".sig") { - continue - } - if strings.HasSuffix(tag, ".sbom") { - continue - } - - tags = append(tags, api.ImageTag{ - Tag: tag, - SHA: sha, - Timestamp: ver.CreatedAt.Time, - }) - } - } - if resp.NextPage == 0 { - break + tags = append(tags, api.ImageTag{ + Tag: tag, + SHA: sha, + Timestamp: ver.CreatedAt.Time, + }) } - - opts.ListOptions.Page = resp.NextPage } + return tags +} - return tags, nil +func (c *Client) shouldSkipTag(tag string) bool { + return strings.HasSuffix(tag, ".att") || + strings.HasSuffix(tag, ".sig") || + strings.HasSuffix(tag, ".sbom") } func (c *Client) ownerType(ctx context.Context, owner string) (string, error) { diff --git a/pkg/client/oci/oci.go b/pkg/client/oci/oci.go index 7ef5fce6..61055735 100644 --- a/pkg/client/oci/oci.go +++ b/pkg/client/oci/oci.go @@ -40,7 +40,7 @@ func (c *Client) Tags(ctx context.Context, host, repo, image string) ([]api.Imag return tags, nil } -func (c *Client) IsHost(host string) bool { +func (c *Client) IsHost(_ string) bool { return true } diff --git a/pkg/client/quay/pager.go b/pkg/client/quay/pager.go index 22a1a678..ef3ce7b6 100644 --- a/pkg/client/quay/pager.go +++ b/pkg/client/quay/pager.go @@ -8,7 +8,7 @@ import ( "github.com/jetstack/version-checker/pkg/api" ) -// pager is used for implementing the paging mechanism for fetching image tags +// pager is used for implementing the paging mechanism for fetching image tags. type pager struct { *Client @@ -53,7 +53,7 @@ func (p *pager) fetchTags(ctx context.Context) error { } // fetchTagsPaged will return the image tags from a given page number, as well -// as if there are more pages +// as if there are more pages. func (p *pager) fetchTagsPaged(ctx context.Context, page int) (bool, error) { url := fmt.Sprintf(tagURL, p.repo, p.image, page) var resp responseTag diff --git a/pkg/client/quay/quay.go b/pkg/client/quay/quay.go index ca6031e9..8b2939f8 100644 --- a/pkg/client/quay/quay.go +++ b/pkg/client/quay/quay.go @@ -72,7 +72,7 @@ func (c *Client) Name() string { return "quay" } -// Fetch the image tags from an upstream repository and image +// Fetch the image tags from an upstream repository and image. func (c *Client) Tags(ctx context.Context, _, repo, image string) ([]api.ImageTag, error) { p := c.newPager(repo, image) @@ -83,7 +83,7 @@ func (c *Client) Tags(ctx context.Context, _, repo, image string) ([]api.ImageTa return p.tags, nil } -// fetchImageManifest will lookup all manifests for a tag, if it is a list +// fetchImageManifest will lookup all manifests for a tag, if it is a list. func (c *Client) fetchImageManifest(ctx context.Context, repo, image string, tag *responseTagItem) ([]api.ImageTag, error) { timestamp, err := time.Parse(time.RFC1123Z, tag.LastModified) if err != nil { @@ -116,7 +116,7 @@ func (c *Client) fetchImageManifest(ctx context.Context, repo, image string, tag }, nil } -// callManifests endpoint on the tags image manifest +// callManifests endpoint on the tags image manifest. func (c *Client) callManifests(ctx context.Context, timestamp time.Time, tag, url string) ([]api.ImageTag, error) { var manifestResp responseManifest if err := c.makeRequest(ctx, url, &manifestResp); err != nil { @@ -167,6 +167,7 @@ func (c *Client) makeRequest(ctx context.Context, url string, obj interface{}) e if err != nil { return fmt.Errorf("failed to make quay call %q: %s", url, err) } + defer resp.Body.Close() if err := json.NewDecoder(resp.Body).Decode(obj); err != nil { return err diff --git a/pkg/client/selfhosted/path.go b/pkg/client/selfhosted/path.go index c61ba5e2..3e444c00 100644 --- a/pkg/client/selfhosted/path.go +++ b/pkg/client/selfhosted/path.go @@ -8,7 +8,7 @@ import ( ) const ( - // Regex template to be used to check "isHost" + // Regex template to be used to check "isHost". hostRegTemplate = `^.*%s$` ) diff --git a/pkg/client/util/util.go b/pkg/client/util/util.go index 14ece724..018a2c62 100644 --- a/pkg/client/util/util.go +++ b/pkg/client/util/util.go @@ -33,7 +33,7 @@ var ( } ) -// Join repo and image strings +// Join repo and image strings. func JoinRepoImage(repo, image string) string { if len(repo) == 0 { return image @@ -45,7 +45,7 @@ func JoinRepoImage(repo, image string) string { return repo + "/" + image } -// Attempt to determine the OS and Arch, given a tag name +// Attempt to determine the OS and Arch, given a tag name. func OSArchFromTag(tag string) (api.OS, api.Architecture) { var ( os api.OS diff --git a/pkg/controller/checker/checker.go b/pkg/controller/checker/checker.go index 2bc59da3..8149dcce 100644 --- a/pkg/controller/checker/checker.go +++ b/pkg/controller/checker/checker.go @@ -30,48 +30,58 @@ func New(search search.Searcher) *Checker { } } -// Container will return the result of the given container's current version, compared to the latest upstream -func (c *Checker) Container(ctx context.Context, log *logrus.Entry, - pod *corev1.Pod, container *corev1.Container, opts *api.Options) (*Result, error) { - - // If the container image SHA status is not ready yet, exit early +// Container will return the result of the given container's current version, compared to the latest upstream. +func (c *Checker) Container(ctx context.Context, log *logrus.Entry, pod *corev1.Pod, + container *corev1.Container, opts *api.Options) (*Result, error) { statusSHA := containerStatusImageSHA(pod, container.Name) if len(statusSHA) == 0 { return nil, nil } imageURL, currentTag, currentSHA := urlTagSHAFromImage(container.Image) + usingSHA, usingTag := len(currentSHA) > 0, len(currentTag) > 0 - usingSHA := len(currentSHA) > 0 - usingTag := len(currentTag) > 0 - - // If using latest or no tag, then compare on SHA if c.isLatestOrEmptyTag(currentTag) { - // Override options to use SHA - opts.UseSHA = true + c.handleLatestOrEmptyTag(log, currentTag, currentSHA, opts) usingTag = false - log.WithField("module", "checker").Debugf("image using %q tag, comparing image SHA %q", - currentTag, currentSHA) } + imageURL = c.overrideImageURL(log, imageURL, opts) + + if opts.UseSHA { + return c.handleSHA(ctx, imageURL, statusSHA, opts, usingTag, currentTag) + } + + return c.handleSemver(ctx, imageURL, statusSHA, currentTag, usingSHA, opts) +} + +func (c *Checker) handleLatestOrEmptyTag(log *logrus.Entry, currentTag, currentSHA string, opts *api.Options) { + opts.UseSHA = true + log.WithField("module", "checker").Debugf("image using %q tag, comparing image SHA %q", currentTag, currentSHA) +} + +func (c *Checker) overrideImageURL(log *logrus.Entry, imageURL string, opts *api.Options) string { if opts.OverrideURL != nil && *opts.OverrideURL != imageURL { log.Debugf("overriding image URL %s -> %s", imageURL, *opts.OverrideURL) - imageURL = *opts.OverrideURL + return *opts.OverrideURL } + return imageURL +} - if opts.UseSHA { - result, err := c.isLatestSHA(ctx, imageURL, statusSHA, opts) - if err != nil { - return nil, err - } - - if usingTag { - result.CurrentVersion = fmt.Sprintf("%s@%s", currentTag, result.CurrentVersion) - } +func (c *Checker) handleSHA(ctx context.Context, imageURL, statusSHA string, opts *api.Options, usingTag bool, currentTag string) (*Result, error) { + result, err := c.isLatestSHA(ctx, imageURL, statusSHA, opts) + if err != nil { + return nil, err + } - return result, nil + if usingTag { + result.CurrentVersion = fmt.Sprintf("%s@%s", currentTag, result.CurrentVersion) } + return result, nil +} + +func (c *Checker) handleSemver(ctx context.Context, imageURL, statusSHA, currentTag string, usingSHA bool, opts *api.Options) (*Result, error) { currentImage := semver.Parse(currentTag) latestImage, isLatest, err := c.isLatestSemver(ctx, imageURL, statusSHA, currentImage, opts) if err != nil { @@ -79,13 +89,10 @@ func (c *Checker) Container(ctx context.Context, log *logrus.Entry, } latestVersion := latestImage.Tag - - // If we are using SHA and tag, make latest version include both if usingSHA && !strings.Contains(latestVersion, "@") && latestImage.SHA != "" { latestVersion = fmt.Sprintf("%s@%s", latestVersion, latestImage.SHA) } - // If latest version contains SHA, include in current version if strings.Contains(latestVersion, "@") { currentTag = fmt.Sprintf("%s@%s", currentTag, statusSHA) } @@ -98,7 +105,7 @@ func (c *Checker) Container(ctx context.Context, log *logrus.Entry, }, nil } -// containerStatusImageSHA will return the containers image SHA, if it is ready +// containerStatusImageSHA will return the containers image SHA, if it is ready. func containerStatusImageSHA(pod *corev1.Pod, containerName string) string { for _, status := range pod.Status.InitContainerStatuses { if status.Name == containerName { @@ -130,12 +137,12 @@ func containerStatusImageSHA(pod *corev1.Pod, containerName string) string { return "" } -// isLatestOrEmptyTag will return true if the given tag is ” or 'latest' +// isLatestOrEmptyTag will return true if the given tag is ” or 'latest'. func (c *Checker) isLatestOrEmptyTag(tag string) bool { return tag == "" || tag == "latest" } -// isLatestSemver will return the latest image, and whether the given image is the latest +// isLatestSemver will return the latest image, and whether the given image is the latest. func (c *Checker) isLatestSemver(ctx context.Context, imageURL, currentSHA string, currentImage *semver.SemVer, opts *api.Options) (*api.ImageTag, bool, error) { latestImage, err := c.search.LatestImage(ctx, imageURL, opts) if err != nil { @@ -161,7 +168,7 @@ func (c *Checker) isLatestSemver(ctx context.Context, imageURL, currentSHA strin return latestImage, isLatest, nil } -// isLatestSHA will return the the result of whether the given image is the latest, according to image SHA +// isLatestSHA will return the the result of whether the given image is the latest, according to image SHA. func (c *Checker) isLatestSHA(ctx context.Context, imageURL, currentSHA string, opts *api.Options) (*Result, error) { latestImage, err := c.search.LatestImage(ctx, imageURL, opts) if err != nil { diff --git a/pkg/controller/checker/checker_test.go b/pkg/controller/checker/checker_test.go index e553929a..ac5dadd6 100644 --- a/pkg/controller/checker/checker_test.go +++ b/pkg/controller/checker/checker_test.go @@ -263,7 +263,6 @@ func TestContainer(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { - checker := New(search.New().With(test.searchResp, nil)) pod := &corev1.Pod{ Status: corev1.PodStatus{ @@ -306,7 +305,7 @@ func TestContainerStatusImageSHA(t *testing.T) { }, "if status with wrong name, then return ''": { status: []corev1.ContainerStatus{ - corev1.ContainerStatus{ + { Name: "foo", ImageID: "123", }, @@ -316,11 +315,11 @@ func TestContainerStatusImageSHA(t *testing.T) { }, "if status with wrong name and correct, then return '456'": { status: []corev1.ContainerStatus{ - corev1.ContainerStatus{ + { Name: "foo", ImageID: "123", }, - corev1.ContainerStatus{ + { Name: "test-name", ImageID: "456", }, @@ -330,15 +329,15 @@ func TestContainerStatusImageSHA(t *testing.T) { }, "if status with multiple status, then return first '456'": { status: []corev1.ContainerStatus{ - corev1.ContainerStatus{ + { Name: "foo", ImageID: "123", }, - corev1.ContainerStatus{ + { Name: "test-name", ImageID: "456", }, - corev1.ContainerStatus{ + { Name: "test-name", ImageID: "789", }, @@ -348,11 +347,11 @@ func TestContainerStatusImageSHA(t *testing.T) { }, "if status with includes URL, then return just SHA": { status: []corev1.ContainerStatus{ - corev1.ContainerStatus{ + { Name: "foo", ImageID: "123", }, - corev1.ContainerStatus{ + { Name: "test-name", ImageID: "localhost:5000/joshvanl/version-checker@sha:456", }, diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 2f63cec4..89917c93 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -35,7 +35,7 @@ type Controller struct { kubeClient kubernetes.Interface podLister corev1listers.PodLister - workqueue workqueue.RateLimitingInterface + workqueue workqueue.TypedRateLimitingInterface[any] scheduledWorkQueue scheduler.ScheduledWorkQueue metrics *metrics.Metrics @@ -52,7 +52,7 @@ func New( log *logrus.Entry, defaultTestAll bool, ) *Controller { - workqueue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) + workqueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()) scheduledWorkQueue := scheduler.NewScheduledWorkQueue(clock.RealClock{}, workqueue.Add) log = log.WithField("module", "controller") @@ -79,7 +79,7 @@ func (c *Controller) Run(ctx context.Context, cacheRefreshRate time.Duration) er sharedInformerFactory := informers.NewSharedInformerFactoryWithOptions(c.kubeClient, time.Second*30) c.podLister = sharedInformerFactory.Core().V1().Pods().Lister() podInformer := sharedInformerFactory.Core().V1().Pods().Informer() - podInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + _, err := podInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.addObject, UpdateFunc: func(old, new interface{}) { if key, err := cache.MetaNamespaceKeyFunc(old); err == nil { @@ -89,6 +89,9 @@ func (c *Controller) Run(ctx context.Context, cacheRefreshRate time.Duration) er }, DeleteFunc: c.deleteObject, }) + if err != nil { + return fmt.Errorf("error creating podInformer: %s", err) + } c.log.Info("starting control loop") sharedInformerFactory.Start(ctx.Done()) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go new file mode 100644 index 00000000..7b9fa729 --- /dev/null +++ b/pkg/controller/controller_test.go @@ -0,0 +1,155 @@ +package controller + +import ( + "context" + "io" + "testing" + "time" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" + + "github.com/jetstack/version-checker/pkg/client" + "github.com/jetstack/version-checker/pkg/metrics" +) + +var testLogger = logrus.NewEntry(logrus.New()) + +func init() { + testLogger.Logger.SetOutput(io.Discard) +} + +func TestNewController(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + + controller := New(5*time.Minute, metrics, imageClient, kubeClient, testLogger, true) + + assert.NotNil(t, controller) + assert.Equal(t, controller.defaultTestAll, true) + assert.NotNil(t, controller.workqueue) + assert.NotNil(t, controller.checker) + assert.NotNil(t, controller.scheduledWorkQueue) +} + +func TestRun(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + controller := New(5*time.Minute, metrics, imageClient, kubeClient, testLogger, true) + + ctx, cancel := context.WithCancel(context.Background()) + + // Run the controller in a separate goroutine so the test doesn't block indefinitely + go func() { + err := controller.Run(ctx, 30*time.Second) + assert.NoError(t, err) + }() + + // Give the controller some time to start up and do initial processing + time.Sleep(1 * time.Second) + + // Cancel the context to stop the controller + cancel() + + // Wait a moment to ensure the controller has shut down + time.Sleep(1 * time.Second) + + // Example assertion: Ensure the Run method has exited (this is implicit by the test not timing out) + assert.True(t, true, "Controller should shutdown cleanly on context cancellation") + // You can also add assertions here if you want to validate any specific state after shutdown + assert.NotNil(t, controller.scheduledWorkQueue, "ScheduledWorkQueue should be initialized") +} + +func TestAddObject(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + controller := New(5*time.Minute, metrics, imageClient, kubeClient, testLogger, true) + + obj := &corev1.Pod{} + controller.addObject(obj) + + // Wait for the item to be added to the workqueue + key, _ := cache.MetaNamespaceKeyFunc(obj) + + // Retry a few times with a short sleep to ensure the item has been added + for i := 0; i < 10; i++ { + if controller.workqueue.Len() > 0 { + break + } + time.Sleep(10 * time.Millisecond) + } + + // Check the workqueue length and the added item + assert.Equal(t, 1, controller.workqueue.Len(), "Expected workqueue to have 1 item after adding an object") + + item, _ := controller.workqueue.Get() + assert.Equal(t, key, item, "Expected the workqueue item to match the object's key") +} + +func TestDeleteObject(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + controller := New(5*time.Minute, metrics, imageClient, kubeClient, testLogger, true) + + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "container1"}, + {Name: "container2"}, + }, + }, + } + controller.deleteObject(pod) + + // We can't directly assert on log messages or metric updates, + // but we can ensure that no errors are thrown and the function executes. + assert.NotNil(t, pod) +} + +func TestProcessNextWorkItem(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + controller := New(5*time.Minute, metrics, imageClient, kubeClient, testLogger, true) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Create a fake pod + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + // Use a fake informer factory and lister + informerFactory := informers.NewSharedInformerFactory(kubeClient, time.Minute) + podInformer := informerFactory.Core().V1().Pods().Informer() + controller.podLister = informerFactory.Core().V1().Pods().Lister() + + // Add the pod to the fake informer + err := podInformer.GetIndexer().Add(pod) + assert.NoError(t, err) + + // Add the pod key to the workqueue + controller.workqueue.AddRateLimited("default/test-pod") + + // Start the informer to process the added pod + informerFactory.Start(ctx.Done()) + cache.WaitForCacheSync(ctx.Done(), podInformer.HasSynced) + + // Test the processNextWorkItem method + err = controller.processNextWorkItem(ctx, "default/test-pod", 30*time.Second) + assert.NoError(t, err) +} diff --git a/pkg/controller/options/options.go b/pkg/controller/options/options.go index 11b9f849..9982f86d 100644 --- a/pkg/controller/options/options.go +++ b/pkg/controller/options/options.go @@ -10,12 +10,14 @@ import ( "github.com/jetstack/version-checker/pkg/api" ) -// Builder is a struct for building container search options +// Builder is a struct for building container search options. type Builder struct { ans map[string]string } -// New contructs a new Builder +type optionsHandler func(name string, opts *api.Options, setNonSha *bool, errs *[]string) error + +// New contructs a new Builder. func New(annotations map[string]string) *Builder { return &Builder{ ans: annotations, @@ -31,92 +33,117 @@ func (b *Builder) Options(name string) (*api.Options, error) { setNonSha bool ) + // Define the handlers + handlers := []optionsHandler{ + b.handleSHAOption, + b.handleMetadataOption, + b.handleRegexOption, + b.handlePinMajorOption, + b.handlePinMinorOption, + b.handlePinPatchOption, + b.handleOverrideURLOption, + } + + // Execute each handler + for _, handler := range handlers { + if err := handler(name, &opts, &setNonSha, &errs); err != nil { + errs = append(errs, err.Error()) + } + } + + // Ensure UseSHA is not used with other semver options + if opts.UseSHA && setNonSha { + errs = append(errs, fmt.Sprintf("cannot define %q with any semver options", b.index(name, api.UseSHAAnnotationKey))) + } + + if len(errs) > 0 { + return nil, errors.New(strings.Join(errs, ", ")) + } + + return &opts, nil +} +func (b *Builder) handleSHAOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if useSHA, ok := b.ans[b.index(name, api.UseSHAAnnotationKey)]; ok && useSHA == "true" { opts.UseSHA = true } + return nil +} +func (b *Builder) handleMetadataOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if useMetaData, ok := b.ans[b.index(name, api.UseMetaDataAnnotationKey)]; ok && useMetaData == "true" { - setNonSha = true + *setNonSha = true opts.UseMetaData = true } + return nil +} +func (b *Builder) handleRegexOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if matchRegex, ok := b.ans[b.index(name, api.MatchRegexAnnotationKey)]; ok { - setNonSha = true + *setNonSha = true opts.MatchRegex = &matchRegex regexMatcher, err := regexp.Compile(matchRegex) if err != nil { - errs = append(errs, fmt.Sprintf("failed to compile regex at annotation %q: %s", - api.MatchRegexAnnotationKey, err)) + *errs = append(*errs, fmt.Sprintf("failed to compile regex at annotation %q: %s", api.MatchRegexAnnotationKey, err)) } else { opts.RegexMatcher = regexMatcher } } + return nil +} +func (b *Builder) handlePinMajorOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if pinMajor, ok := b.ans[b.index(name, api.PinMajorAnnotationKey)]; ok { - setNonSha = true - + *setNonSha = true ma, err := strconv.ParseInt(pinMajor, 10, 64) if err != nil { - errs = append(errs, fmt.Sprintf("failed to parse %s: %s", - b.index(name, api.PinMajorAnnotationKey), err)) + *errs = append(*errs, fmt.Sprintf("failed to parse %s: %s", b.index(name, api.PinMajorAnnotationKey), err)) } else { opts.PinMajor = &ma } } + return nil +} +func (b *Builder) handlePinMinorOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if pinMinor, ok := b.ans[b.index(name, api.PinMinorAnnotationKey)]; ok { - setNonSha = true - + *setNonSha = true if opts.PinMajor == nil { - errs = append(errs, fmt.Sprintf("unable to set %q without setting %q", - b.index(name, api.PinMinorAnnotationKey), b.index(name, api.PinMajorAnnotationKey))) + *errs = append(*errs, fmt.Sprintf("unable to set %q without setting %q", b.index(name, api.PinMinorAnnotationKey), b.index(name, api.PinMajorAnnotationKey))) } else { - mi, err := strconv.ParseInt(pinMinor, 10, 64) if err != nil { - errs = append(errs, fmt.Sprintf("failed to parse %s: %s", - b.index(name, api.PinMinorAnnotationKey), err)) + *errs = append(*errs, fmt.Sprintf("failed to parse %s: %s", b.index(name, api.PinMinorAnnotationKey), err)) } else { opts.PinMinor = &mi } } } + return nil +} +func (b *Builder) handlePinPatchOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if pinPatch, ok := b.ans[b.index(name, api.PinPatchAnnotationKey)]; ok { - setNonSha = true - - if opts.PinMajor == nil && opts.PinMinor == nil { - errs = append(errs, fmt.Sprintf("unable to set %q without setting %q and %q", - b.index(name, api.PinPatchAnnotationKey), - b.index(name, api.PinMinorAnnotationKey), - b.index(name, api.PinMajorAnnotationKey))) + *setNonSha = true + if opts.PinMajor == nil || opts.PinMinor == nil { + *errs = append(*errs, fmt.Sprintf("unable to set %q without setting %q and %q", b.index(name, api.PinPatchAnnotationKey), b.index(name, api.PinMinorAnnotationKey), b.index(name, api.PinMajorAnnotationKey))) } else { - pa, err := strconv.ParseInt(pinPatch, 10, 64) if err != nil { - errs = append(errs, fmt.Sprintf("failed to parse %s: %s", - b.index(name, api.PinPatchAnnotationKey), err)) + *errs = append(*errs, fmt.Sprintf("failed to parse %s: %s", b.index(name, api.PinPatchAnnotationKey), err)) } else { opts.PinPatch = &pa } } } + return nil +} +func (b *Builder) handleOverrideURLOption(name string, opts *api.Options, setNonSha *bool, errs *[]string) error { if overrideURL, ok := b.ans[b.index(name, api.OverrideURLAnnotationKey)]; ok { opts.OverrideURL = &overrideURL } - - if opts.UseSHA && setNonSha { - errs = append(errs, fmt.Sprintf("cannot define %q with any semver otions", - b.index(name, api.UseSHAAnnotationKey))) - } - - if len(errs) > 0 { - return nil, errors.New(strings.Join(errs, ", ")) - } - - return &opts, nil + return nil } // IsEnabled will return whether the container has the enabled annotation set. @@ -132,7 +159,7 @@ func (b *Builder) IsEnabled(defaultEnabled bool, name string) bool { } } -// index returns the annotation index give the API annotaion key +// index returns the annotation index give the API annotaion key. func (b *Builder) index(containerName, annotationName string) string { return annotationName + "/" + containerName } diff --git a/pkg/controller/options/options_test.go b/pkg/controller/options/options_test.go index fc817ca6..f54c102a 100644 --- a/pkg/controller/options/options_test.go +++ b/pkg/controller/options/options_test.go @@ -60,7 +60,7 @@ func TestBuild(t *testing.T) { api.UseSHAAnnotationKey + "/test-name": "true", }, expOptions: nil, - expErr: `cannot define "use-sha.version-checker.io/test-name" with any semver otions`, + expErr: `cannot define "use-sha.version-checker.io/test-name" with any semver options`, }, "cannot use sha with non sha options (pins)": { containerName: "test-name", @@ -70,7 +70,7 @@ func TestBuild(t *testing.T) { api.UseSHAAnnotationKey + "/test-name": "true", }, expOptions: nil, - expErr: `cannot define "use-sha.version-checker.io/test-name" with any semver otions`, + expErr: `cannot define "use-sha.version-checker.io/test-name" with any semver options`, }, "output options for pins and add metadata": { containerName: "test-name", diff --git a/pkg/controller/scheduler/scheduler.go b/pkg/controller/scheduler/scheduler.go index 130b9389..c070abf4 100644 --- a/pkg/controller/scheduler/scheduler.go +++ b/pkg/controller/scheduler/scheduler.go @@ -25,7 +25,7 @@ var afterFunc = func(c clock.Clock, d time.Duration, f func()) stoppable { return t } -// stoppable is the subset of time.Timer which we use, split out for mocking purposes +// stoppable is the subset of time.Timer which we use, split out for mocking purposes. type stoppable interface { Stop() bool } @@ -52,7 +52,7 @@ type scheduledWorkQueue struct { workLock sync.Mutex } -// NewScheduledWorkQueue will create a new workqueue with the given processFunc +// NewScheduledWorkQueue will create a new workqueue with the given processFunc. func NewScheduledWorkQueue(clock clock.Clock, processFunc ProcessFunc) ScheduledWorkQueue { return &scheduledWorkQueue{ processFunc: processFunc, @@ -82,7 +82,7 @@ func (s *scheduledWorkQueue) Forget(obj interface{}) { s.forget(obj) } -// forget cancels and removes an item. It *must* be called with the lock already held +// forget cancels and removes an item. It *must* be called with the lock already held. func (s *scheduledWorkQueue) forget(obj interface{}) { if timer, ok := s.work[obj]; ok { timer.Stop() diff --git a/pkg/controller/scheduler/scheduler_test.go b/pkg/controller/scheduler/scheduler_test.go index a17d287e..0e50d508 100644 --- a/pkg/controller/scheduler/scheduler_test.go +++ b/pkg/controller/scheduler/scheduler_test.go @@ -69,7 +69,7 @@ func TestForget(t *testing.T) { t.Run(test.obj, func(test testT) func(*testing.T) { return func(t *testing.T) { defer wg.Done() - queue := NewScheduledWorkQueue(clock.RealClock{}, func(obj interface{}) { + queue := NewScheduledWorkQueue(clock.RealClock{}, func(_ interface{}) { t.Errorf("scheduled function should never be called") }) queue.Add(test.obj, test.duration) @@ -131,7 +131,7 @@ func newMockAfter() *mockAfter { } } -func (m *mockAfter) AfterFunc(c clock.Clock, d time.Duration, f func()) stoppable { +func (m *mockAfter) AfterFunc(_ clock.Clock, d time.Duration, f func()) stoppable { m.lock.Lock() defer m.lock.Unlock() diff --git a/pkg/controller/search/search.go b/pkg/controller/search/search.go index 43e3086c..f5f80a26 100644 --- a/pkg/controller/search/search.go +++ b/pkg/controller/search/search.go @@ -14,7 +14,7 @@ import ( "github.com/jetstack/version-checker/pkg/version" ) -// Searcher is the interface for Search to facilitate testing +// Searcher is the interface for Search to facilitate testing. type Searcher interface { Run(time.Duration) LatestImage(context.Context, string, *api.Options) (*api.ImageTag, error) @@ -28,7 +28,7 @@ type Search struct { searchCache *cache.Cache } -// New creates a new Search for querying searches over image tags +// New creates a new Search for querying searches over image tags. func New(log *logrus.Entry, cacheTimeout time.Duration, versionGetter *version.Version) *Search { s := &Search{ log: log.WithField("module", "search"), diff --git a/pkg/controller/sync.go b/pkg/controller/sync.go index 6b5a33ae..d422acc6 100644 --- a/pkg/controller/sync.go +++ b/pkg/controller/sync.go @@ -39,7 +39,7 @@ func (c *Controller) sync(ctx context.Context, pod *corev1.Pod) error { return nil } -// syncContainer will enqueue a given container to check the version +// syncContainer will enqueue a given container to check the version. func (c *Controller) syncContainer(ctx context.Context, log *logrus.Entry, builder *options.Builder, pod *corev1.Pod, container *corev1.Container, containerType string) error { // If not enabled, exit early diff --git a/pkg/controller/sync_test.go b/pkg/controller/sync_test.go new file mode 100644 index 00000000..ba1ade4e --- /dev/null +++ b/pkg/controller/sync_test.go @@ -0,0 +1,143 @@ +package controller + +import ( + "context" + "testing" + "time" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/jetstack/version-checker/pkg/api" + "github.com/jetstack/version-checker/pkg/client" + "github.com/jetstack/version-checker/pkg/controller/checker" + "github.com/jetstack/version-checker/pkg/controller/options" + "github.com/jetstack/version-checker/pkg/controller/search" + "github.com/jetstack/version-checker/pkg/metrics" + "github.com/jetstack/version-checker/pkg/version" +) + +// Test for the sync method. +func TestController_Sync(t *testing.T) { + log := logrus.NewEntry(logrus.New()) + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + searcher := search.New(log, 5*time.Minute, version.New(log, imageClient, 5*time.Minute)) + checker := checker.New(searcher) + + controller := &Controller{ + log: log, + checker: checker, + metrics: metrics, + defaultTestAll: true, + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "init-container"}, + }, + Containers: []corev1.Container{ + {Name: "main-container"}, + }, + }, + } + + err := controller.sync(context.Background(), pod) + assert.NoError(t, err) +} + +// Test for the syncContainer method. +func TestController_SyncContainer(t *testing.T) { + log := logrus.NewEntry(logrus.New()) + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + searcher := search.New(log, 5*time.Minute, version.New(log, imageClient, 5*time.Minute)) + checker := checker.New(searcher) + + controller := &Controller{ + log: log, + checker: checker, + metrics: metrics, + defaultTestAll: true, + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + container := &corev1.Container{Name: "main-container"} + + builder := options.New(map[string]string{ + "version-checker.jetstack.io/enabled": "true", + }) + + err := controller.syncContainer(context.Background(), log, builder, pod, container, "container") + assert.NoError(t, err) +} + +// Test for the checkContainer method. +func TestController_CheckContainer(t *testing.T) { + log := logrus.NewEntry(logrus.New()) + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + searcher := search.New(log, 5*time.Minute, version.New(log, imageClient, 5*time.Minute)) + checker := checker.New(searcher) + + controller := &Controller{ + log: log, + checker: checker, + metrics: metrics, + defaultTestAll: true, + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + container := &corev1.Container{Name: "main-container"} + opts := &api.Options{} + + err := controller.checkContainer(context.Background(), log, pod, container, "container", opts) + assert.NoError(t, err) +} + +// Example of testing syncContainer when version is not found. +func TestController_SyncContainer_NoVersionFound(t *testing.T) { + log := logrus.NewEntry(logrus.New()) + metrics := &metrics.Metrics{} + imageClient := &client.Client{} + searcher := search.New(log, 5*time.Minute, version.New(log, imageClient, 5*time.Minute)) + checker := checker.New(searcher) + + controller := &Controller{ + log: log, + checker: checker, + metrics: metrics, + defaultTestAll: true, + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + container := &corev1.Container{Name: "main-container"} + builder := options.New(map[string]string{ + "version-checker.jetstack.io/enabled": "true", + }) + + err := controller.syncContainer(context.Background(), log, builder, pod, container, "container") + assert.NoError(t, err) // We expect no error because IsNoVersionFound is handled gracefully +} diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index ba99bcce..090baed6 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -21,7 +21,6 @@ import ( type Metrics struct { *http.Server - registry *prometheus.Registry containerImageVersion *prometheus.GaugeVec log *logrus.Entry @@ -56,7 +55,7 @@ func New(log *logrus.Entry) *Metrics { } } -// Run will run the metrics server +// Run will run the metrics server. func (m *Metrics) Run(servingAddress string) error { router := http.NewServeMux() router.Handle("/metrics", promhttp.Handler()) @@ -171,7 +170,7 @@ func (m *Metrics) Shutdown() error { return nil } -func (m *Metrics) healthzAndReadyzHandler(w http.ResponseWriter, r *http.Request) { +func (m *Metrics) healthzAndReadyzHandler(w http.ResponseWriter, _ *http.Request) { // Its not great, but does help ensure that we're alive and ready over // calling the /metrics endpoint which can be expensive on large payloads _, err := w.Write([]byte("OK")) diff --git a/pkg/version/semver/semver.go b/pkg/version/semver/semver.go index 24ae687e..0fb35d9a 100644 --- a/pkg/version/semver/semver.go +++ b/pkg/version/semver/semver.go @@ -48,25 +48,44 @@ func Parse(tag string) *SemVer { // LessThan will return true if the given semver is equal, or larger that the // calling semver. If the calling SemVer has metadata, then ASCII comparison // will take place on the version. -// e.g. v1.0.1-alpha.1 < v1.0.1-beta.0 +// e.g. v1.0.1-alpha.1 < v1.0.1-beta.0. func (s *SemVer) LessThan(other *SemVer) bool { - if len(other.original) == 0 || len(s.original) == 0 { + if s.isInvalidComparison(other) { return len(s.original) < len(other.original) } - // if s doesn't have metadata but other doesn't, false. + // Compare stable vs. pre-release if !s.HasMetaData() && other.HasMetaData() { return false } + if s.HasMetaData() && !other.HasMetaData() { + return true + } + + // Compare version numbers + if s.compareVersionNumbers(other) { + return true + } + // Compare pre-release metadata + return s.comparePreReleaseMetadata(other) +} +func (s *SemVer) isInvalidComparison(other *SemVer) bool { + return len(other.original) == 0 || len(s.original) == 0 +} +func (s *SemVer) compareVersionNumbers(other *SemVer) bool { for i := 0; i < 3; i++ { if s.version[i] != other.version[i] { return s.version[i] < other.version[i] } } + return false +} +func (s *SemVer) comparePreReleaseMetadata(other *SemVer) bool { sWords := parseStringToWords(s.metadata) otherWords := parseStringToWords(other.metadata) + l := len(sWords) if len(otherWords) > l { l = len(otherWords) @@ -97,7 +116,7 @@ func (s *SemVer) Equal(other *SemVer) bool { // HasMetaData returns whether this SemVer has metadata. MetaData is defined // as a tag containing anything after the patch digit. -// e.g. v1.0.1-gke.3, v1.0.1-alpha.0, v1.2.3.4 +// e.g. v1.0.1-gke.3, v1.0.1-alpha.0, v1.2.3.4. func (s *SemVer) HasMetaData() bool { return len(s.metadata) > 0 } diff --git a/pkg/version/version.go b/pkg/version/version.go index f17676ae..db0f33db 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -63,7 +63,6 @@ func (v *Version) LatestTagFromImage(ctx context.Context, imageURL string, opts return nil, versionerrors.NewVersionErrorNotFound("%s: failed to find latest image based on SHA", imageURL) } - } else { tag, err = latestSemver(opts, tags) if err != nil { @@ -110,39 +109,11 @@ func latestSemver(opts *api.Options, tags []api.ImageTag) (*api.ImageTag, error) for i := range tags { v := semver.Parse(tags[i].Tag) - // If regex enabled continue here. - // If we match, and is less than, update latest. - if opts.RegexMatcher != nil { - if opts.RegexMatcher.MatchString(tags[i].Tag) && - (latestV == nil || latestV.LessThan(v)) { - latestV = v - latestImageTag = &tags[i] - } - - continue - } - - // If we have declared we wont use metadata but version has it, continue. - if !opts.UseMetaData && v.HasMetaData() { - continue - } - - if opts.PinMajor != nil && *opts.PinMajor != v.Major() { - continue - } - if opts.PinMinor != nil && *opts.PinMinor != v.Minor() { - continue - } - if opts.PinPatch != nil && *opts.PinPatch != v.Patch() { + if shouldSkipTag(opts, v) { continue } - // If no latest yet set - if latestV == nil || - // If the latest set is less than - latestV.LessThan(v) || - // If the latest is the same tag, but smaller timestamp - (latestV.Equal(v) && tags[i].Timestamp.After(latestImageTag.Timestamp)) { + if isBetterTag(opts, latestV, v, latestImageTag, &tags[i]) { latestV = v latestImageTag = &tags[i] } @@ -155,6 +126,38 @@ func latestSemver(opts *api.Options, tags []api.ImageTag) (*api.ImageTag, error) return latestImageTag, nil } +func shouldSkipTag(opts *api.Options, v *semver.SemVer) bool { + // Handle Regex matching + if opts.RegexMatcher != nil { + return !opts.RegexMatcher.MatchString(v.String()) + } + + // Handle metadata and version pinning + return (!opts.UseMetaData && v.HasMetaData()) || + (opts.PinMajor != nil && *opts.PinMajor != v.Major()) || + (opts.PinMinor != nil && *opts.PinMinor != v.Minor()) || + (opts.PinPatch != nil && *opts.PinPatch != v.Patch()) +} + +func isBetterTag(_ *api.Options, latestV, v *semver.SemVer, latestImageTag, currentImageTag *api.ImageTag) bool { + // No latest version set yet + if latestV == nil { + return true + } + + // If the current version is greater than the latest + if latestV.LessThan(v) { + return true + } + + // If the versions are equal, prefer the one with a later timestamp + if latestV.Equal(v) && currentImageTag.Timestamp.After(latestImageTag.Timestamp) { + return true + } + + return false +} + // latestSHA will return the latest ImageTag based on image timestamps. func latestSHA(tags []api.ImageTag) (*api.ImageTag, error) { var latestTag *api.ImageTag diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index 6f744ecd..22aa6b70 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" ) -// Helper function to parse time +// Helper function to parse time. func parseTime(t string) time.Time { parsedTime, _ := time.Parse(time.RFC3339, t) return parsedTime @@ -181,7 +181,7 @@ func TestLatestSemver(t *testing.T) { { name: "Alpha/Beta SemVer", opts: &api.Options{ - UseMetaData: true, + UseMetaData: false, }, tags: alphaBetaTags, expected: "v1.1.1",