Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ func main() {
os.Exit(1)
}

versionUpdater := controller.NewVersionUpdater(versionGetter, imageValidators, maintenanceTriggers, baseImage)
authVersion := version.NewAuthVersionGetter(mgr.GetClient())

versionUpdater := controller.NewVersionUpdater(versionGetter, imageValidators, maintenanceTriggers, baseImage, authVersion)

// Controller registration
deploymentController := controller.DeploymentVersionUpdater{
Expand Down
8 changes: 6 additions & 2 deletions integrations/kube-agent-updater/pkg/controller/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ func (r *DeploymentVersionUpdater) Reconcile(ctx context.Context, req ctrl.Reque

image, err := r.GetVersion(ctx, &obj, currentVersion)
var (
noNewVersionErr *NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
noNewVersionErr *NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
incompatibleVersionErr *IncompatibleVersionError
)
switch {
case errors.As(err, &noNewVersionErr):
Expand All @@ -85,6 +86,9 @@ func (r *DeploymentVersionUpdater) Reconcile(ctx context.Context, req ctrl.Reque
// Not logging the error because it provides no other information than its type.
log.Info("No maintenance triggered, not updating.", "currentVersion", currentVersion)
return requeueLater, nil
case errors.As(err, &incompatibleVersionErr):
log.Info("Target version is incompatible with the auth server version.")
return requeueLater, nil
case trace.IsTrustError(err):
// Logging as error as image verification should not fail under normal use
log.Error(err, "Image verification failed, not updating.")
Expand Down
15 changes: 15 additions & 0 deletions integrations/kube-agent-updater/pkg/controller/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,18 @@ func (e *NoNewVersionError) Error() string {
}
return fmt.Sprintf("no new version (current: %q, next: %q)", e.CurrentVersion, e.NextVersion)
}

// IncompatibleVersionError indicates that the target version is incompatible with the auth server version
type IncompatibleVersionError struct {
Message string `json:"message"`
AuthVersion string `json:"authVersion"`
NextVersion string `json:"nextVersion"`
}

// Error returns a log friendly description of an error
func (e *IncompatibleVersionError) Error() string {
if e.Message != "" {
return e.Message
}
return fmt.Sprintf("next version is incompatible with auth version (auth: %q, next: %q)", e.AuthVersion, e.NextVersion)
}
8 changes: 6 additions & 2 deletions integrations/kube-agent-updater/pkg/controller/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ func (r *StatefulSetVersionUpdater) Reconcile(ctx context.Context, req ctrl.Requ

image, err := r.GetVersion(ctx, &obj, currentVersion)
var (
noNewVersionErr *NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
noNewVersionErr *NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
incompatibleVersionErr *IncompatibleVersionError
)
switch {
case errors.As(err, &noNewVersionErr):
Expand All @@ -115,6 +116,9 @@ func (r *StatefulSetVersionUpdater) Reconcile(ctx context.Context, req ctrl.Requ
// No need to check for blocked rollout because the unhealthy workload
// trigger has not approved the maintenance
return requeueLater, nil
case errors.As(err, &incompatibleVersionErr):
log.Info("Target version is incompatible with the auth server version.")
return requeueLater, nil
case trace.IsTrustError(err):
// Logging as error as image verification should not fail under normal use
log.Error(err, "Image verification failed, not updating.")
Expand Down
17 changes: 16 additions & 1 deletion integrations/kube-agent-updater/pkg/controller/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/distribution/reference"
"github.com/gravitational/trace"
"golang.org/x/mod/semver"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"

Expand All @@ -35,6 +36,7 @@ type VersionUpdater struct {
imageValidators img.Validators
maintenanceTriggers maintenance.Triggers
baseImage reference.Named
authVersionGetter version.AuthVersionGetter
}

// GetVersion does all the version update logic: checking if a maintenance is allowed,
Expand Down Expand Up @@ -64,6 +66,18 @@ func (r *VersionUpdater) GetVersion(ctx context.Context, obj client.Object, curr
return nil, &NoNewVersionError{CurrentVersion: currentVersion, NextVersion: nextVersion}
}

log.Info("Getting auth server version")
authVersion, err := r.authVersionGetter.Get(ctx, obj)
if err != nil {
return nil, trace.Wrap(err)
}

log.Info("Verify new version candidate is compatible with the auth server version")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add the versions as fields to the log message?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here: adding the versions to the log

// The auth server is incompatible with clients of a newer major version
if semver.Compare(semver.Major(nextVersion), semver.Major(authVersion)) > 0 {
return nil, &IncompatibleVersionError{AuthVersion: authVersion, NextVersion: nextVersion}
}

log.Info("Version change is valid, building img candidate")
// We tag our img candidate with the version
image, err := reference.WithTag(r.baseImage, strings.TrimPrefix(nextVersion, "v"))
Expand All @@ -84,12 +98,13 @@ func (r *VersionUpdater) GetVersion(ctx context.Context, obj client.Object, curr

// NewVersionUpdater returns a version updater using the given version.Getter,
// img.Validators, maintenance.Triggers and baseImage.
func NewVersionUpdater(v version.Getter, i img.Validators, t maintenance.Triggers, b reference.Named) VersionUpdater {
func NewVersionUpdater(v version.Getter, i img.Validators, t maintenance.Triggers, b reference.Named, a version.AuthVersionGetter) VersionUpdater {
// TODO: do checks to see if not nil/empty ?
return VersionUpdater{
versionGetter: v,
imageValidators: i,
maintenanceTriggers: t,
baseImage: b,
authVersionGetter: a,
}
}
20 changes: 20 additions & 0 deletions integrations/kube-agent-updater/pkg/controller/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter version.Getter
maintenanceTriggers []maintenance.Trigger
imageCheckers []img.Validator
authVersionGetter version.AuthVersionGetter
assertErr require.ErrorAssertionFunc
expectedImage string
}{
Expand All @@ -83,6 +84,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock(versionHigh, nil),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{alwaysValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionHigh, nil),
assertErr: require.NoError,
expectedImage: fmt.Sprintf("%s/%s:%s@%s", defaultTestRegistry, defaultTestPath, versionHigh, defaultImageDigest),
},
Expand All @@ -94,6 +96,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock(versionHigh, nil),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{alwaysValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionHigh, nil),
assertErr: require.NoError,
expectedImage: fmt.Sprintf("%s/%s:%s@%s", defaultTestRegistry, defaultTestPath, versionHigh, defaultImageDigest),
},
Expand All @@ -105,6 +108,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock(versionMid, nil),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{alwaysValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionMid, nil),
assertErr: errorIsType(&NoNewVersionError{}),
expectedImage: "",
},
Expand All @@ -116,6 +120,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock(versionHigh, nil),
maintenanceTriggers: []maintenance.Trigger{neverTrigger},
imageCheckers: []img.Validator{alwaysValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionHigh, nil),
assertErr: errorIsType(&MaintenanceNotTriggeredError{}),
expectedImage: "",
},
Expand All @@ -127,6 +132,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock(versionHigh, nil),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{neverValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionHigh, nil),
assertErr: errorIsType(&trace.TrustError{}),
expectedImage: "",
},
Expand All @@ -138,9 +144,22 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
versionGetter: version.NewGetterMock("", &trace.ConnectionProblemError{}),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{neverValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionHigh, nil),
assertErr: errorIsType(&trace.ConnectionProblemError{}),
expectedImage: "",
},
{
name: "target version incompatible with auth version",
releaseRegistry: defaultTestRegistry,
releasePath: defaultTestPath,
currentVersion: versionMid,
versionGetter: version.NewGetterMock(versionHigh, nil),
maintenanceTriggers: []maintenance.Trigger{alwaysTrigger},
imageCheckers: []img.Validator{neverValid},
authVersionGetter: version.NewMockAuthVersionGetter(versionMid, nil),
assertErr: errorIsType(&IncompatibleVersionError{}),
expectedImage: "",
},
}

for _, tt := range tests {
Expand All @@ -157,6 +176,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) {
imageValidators: tt.imageCheckers,
maintenanceTriggers: tt.maintenanceTriggers,
baseImage: baseImage,
authVersionGetter: tt.authVersionGetter,
}

// We need a dummy Kubernetes object, it is not used by the TriggerMock
Expand Down
61 changes: 61 additions & 0 deletions integrations/kube-agent-updater/pkg/version/auth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
Copyright 2023 Gravitational, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package version

import (
"context"
"fmt"

"github.com/gravitational/trace"
v1 "k8s.io/api/core/v1"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
)

const authVersionKeyName = "agent-auth-version"

// AuthVersionGetter gets the auth server version.
type AuthVersionGetter interface {
Get(context.Context, kclient.Object) (string, error)
}

type authVersionGetter struct {
kclient.Client
}

// NewAuthVersionGetter creates a new AuthVersionGetter
func NewAuthVersionGetter(client kclient.Client) AuthVersionGetter {
return &authVersionGetter{Client: client}
}

// Get returns the auth version stored in the shared state secret
func (a *authVersionGetter) Get(ctx context.Context, object kclient.Object) (string, error) {
secretName := fmt.Sprintf("%s-shared-state", object.GetName())
var secret v1.Secret
err := a.Client.Get(ctx, kclient.ObjectKey{Namespace: object.GetNamespace(), Name: secretName}, &secret)
if err != nil {
return "", trace.Wrap(err)
}
rawData, ok := secret.Data[authVersionKeyName]
if !ok {
return "", trace.Errorf("secret %s does not have key %s", secretName, authVersionKeyName)
}
version, err := EnsureSemver(string(rawData))
if err != nil {
return "", trace.Wrap(err)
}
return version, nil
}
111 changes: 111 additions & 0 deletions integrations/kube-agent-updater/pkg/version/auth_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2023 Gravitational, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package version

import (
"context"
"testing"

"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestAuthVersionGetter_Get(t *testing.T) {
// Test setup: generating and loading fixtures
ctx := context.Background()
namespace := "bar"

fixtures := &v1.SecretList{Items: []v1.Secret{
{
ObjectMeta: metav1.ObjectMeta{Name: "no-key-shared-state", Namespace: namespace},
Data: map[string][]byte{"foo": []byte("bar")},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "invalid-json-shared-state", Namespace: namespace},
Data: map[string][]byte{authVersionKeyName: []byte(`{"foo": "bar"}`)},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "invalid-auth-version-shared-state", Namespace: namespace},
Data: map[string][]byte{authVersionKeyName: []byte(".13")},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "valid-auth-version-shared-state", Namespace: namespace},
Data: map[string][]byte{authVersionKeyName: []byte("13.4.5")},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "prefix-shared-state", Namespace: namespace},
Data: map[string][]byte{authVersionKeyName: []byte("v13.4.5")},
},
}}

clientBuilder := fake.NewClientBuilder()
clientBuilder.WithLists(fixtures)
client := clientBuilder.Build()

tests := []struct {
name string
object kclient.Object
want string
assertErr require.ErrorAssertionFunc
}{
{
name: "no secret",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "not-found", Namespace: namespace}},
assertErr: require.Error,
},
{
name: "secret no key",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "no-key", Namespace: namespace}},
assertErr: require.Error,
},
{
name: "secret invalid JSON",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "invalid-json", Namespace: namespace}},
assertErr: require.Error,
},
{
name: "secret invalid auth version",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "invalid-auth-version", Namespace: namespace}},
assertErr: require.Error,
},
{
name: "valid auth version",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "valid-auth-version", Namespace: namespace}},
want: "v13.4.5",
assertErr: require.NoError,
},
{
name: "prefix auth version",
object: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "prefix", Namespace: namespace}},
want: "v13.4.5",
assertErr: require.NoError,
},
}
// Doing the real test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Doing the real test

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
authVersionGetter := NewAuthVersionGetter(client)
version, err := authVersionGetter.Get(ctx, tt.object)
tt.assertErr(t, err)
require.Equal(t, tt.want, version)
})
}
}
Loading