Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom

✨ Add PullSecret controller to save pull secret data locally #1322

Merged
merged 2 commits into from
Oct 8, 2024
Merged

✨ Add PullSecret controller to save pull secret data locally #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eac1e69
✨ Add PullSecret controller to save pull secret data locally
anik120 Sep 30, 2024
018510e
main.go: improved cache configuration for watching pull secret
anik120 Oct 8, 2024
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
101 changes: 72 additions & 29 deletions cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ import (
"net/http"
"os"
"path/filepath"
"strings"
"time"

"github.com/containers/image/v5/types"
"github.com/go-logr/logr"
"github.com/spf13/pflag"
corev1 "k8s.io/api/core/v1"
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
"k8s.io/apimachinery/pkg/fields"
k8slabels "k8s.io/apimachinery/pkg/labels"
k8stypes "k8s.io/apimachinery/pkg/types"
apimachineryrand "k8s.io/apimachinery/pkg/util/rand"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -67,7 +72,7 @@ var (
defaultSystemNamespace = "olmv1-system"
)

const authFilePath = "/etc/operator-controller/auth.json"
const authFilePrefix = "operator-controller-global-pull-secrets"

// podNamespace checks whether the controller is running in a Pod vs.
// being run locally by inspecting the namespace file that gets mounted
Expand All @@ -90,6 +95,7 @@ func main() {
operatorControllerVersion bool
systemNamespace string
caCertDir string
globalPullSecret string
)
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
Expand All @@ -100,6 +106,7 @@ func main() {
flag.StringVar(&cachePath, "cache-path", "/var/cache", "The local directory path used for filesystem based caching")
flag.BoolVar(&operatorControllerVersion, "version", false, "Prints operator-controller version information")
flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
flag.StringVar(&globalPullSecret, "global-pull-secret", "", "The <namespace>/<name> of the global pull secret that is going to be used to pull bundle images.")

klog.InitFlags(flag.CommandLine)

Expand All @@ -116,27 +123,51 @@ func main() {

setupLog.Info("starting up the controller", "version info", version.String())

authFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("%s-%s.json", authFilePrefix, apimachineryrand.String(8)))
var globalPullSecretKey *k8stypes.NamespacedName
if globalPullSecret != "" {
secretParts := strings.Split(globalPullSecret, "/")
anik120 marked this conversation as resolved.
Show resolved Hide resolved
if len(secretParts) != 2 {
setupLog.Error(fmt.Errorf("incorrect number of components"), "value of global-pull-secret should be of the format <namespace>/<name>")
os.Exit(1)
}
globalPullSecretKey = &k8stypes.NamespacedName{Name: secretParts[1], Namespace: secretParts[0]}
}

if systemNamespace == "" {
systemNamespace = podNamespace()
}

setupLog.Info("set up manager")
cacheOptions := crcache.Options{
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: If you can add some comments around what the below code doing it will add a lot to the readability of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not convinced any further documentation is necessary. The crcache.Options struct is pretty well documented. Once the reader understands what the controller-runtime cache does and what Options can be used to configure, the code reads "configuring caching options for the objects we own + additional object Secret in a specific namespace".

ByObject: map[client.Object]crcache.ByObject{
&ocv1alpha1.ClusterExtension{}: {Label: k8slabels.Everything()},
&catalogd.ClusterCatalog{}: {Label: k8slabels.Everything()},
},
DefaultNamespaces: map[string]crcache.Config{
systemNamespace: {LabelSelector: k8slabels.Everything()},
},
DefaultLabelSelector: k8slabels.Nothing(),
}
if globalPullSecretKey != nil {
cacheOptions.ByObject[&corev1.Secret{}] = crcache.ByObject{
Namespaces: map[string]crcache.Config{
globalPullSecretKey.Namespace: {
LabelSelector: k8slabels.Everything(),
FieldSelector: fields.SelectorFromSet(map[string]string{
"metadata.name": globalPullSecretKey.Name,
}),
},
},
}
Comment on lines +153 to +162
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this also override the configuration for secrets in the systemNamespace? IIUC we want to make sure we have access to all the release secrets that get created by the secret driver we use for partitioning helm release secrets for larger bundles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Approved pending an answer to this

Copy link
Member

Choose a reason for hiding this comment

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

Dirty secret: iirc, the release secret driver is still a live client. So checking whether that is working won't actually tell you anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I don't actually see any configuration for secretes in the systemNamespace. There's configuration for &ocv1alpha1.ClusterExtension{} and &catalogd.ClusterCatalog{}, but nothing for Secrets.

}
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: scheme.Scheme,
Metrics: server.Options{BindAddress: metricsAddr},
HealthProbeBindAddress: probeAddr,
LeaderElection: enableLeaderElection,
LeaderElectionID: "9c4404e7.operatorframework.io",
Cache: crcache.Options{
ByObject: map[client.Object]crcache.ByObject{
&ocv1alpha1.ClusterExtension{}: {Label: k8slabels.Everything()},
&catalogd.ClusterCatalog{}: {Label: k8slabels.Everything()},
},
DefaultNamespaces: map[string]crcache.Config{
systemNamespace: {LabelSelector: k8slabels.Everything()},
},
DefaultLabelSelector: k8slabels.Nothing(),
},
Cache: cacheOptions,
// LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily
// when the Manager ends. This requires the binary to immediately end when the
// Manager is stopped, otherwise, this setting is unsafe. Setting this significantly
Expand Down Expand Up @@ -191,12 +222,21 @@ func main() {

unpacker := &source.ContainersImageRegistry{
BaseCachePath: filepath.Join(cachePath, "unpack"),
SourceContext: &types.SystemContext{
DockerCertPath: caCertDir,
OCICertPath: caCertDir,
AuthFilePath: authFilePathIfPresent(setupLog),
},
}
SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) {
srcContext := &types.SystemContext{
DockerCertPath: caCertDir,
OCICertPath: caCertDir,
}
if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil {
logger.Info("using available authentication information for pulling image")
srcContext.AuthFilePath = authFilePath
} else if os.IsNotExist(err) {
logger.Info("no authentication information found for pulling image, proceeding without auth")
LalatenduMohanty marked this conversation as resolved.
Show resolved Hide resolved
} else {
return nil, fmt.Errorf("could not stat auth file, error: %w", err)
}
return srcContext, nil
}}

clusterExtensionFinalizers := crfinalizer.NewFinalizers()
if err := clusterExtensionFinalizers.Register(controllers.ClusterExtensionCleanupUnpackCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
Expand Down Expand Up @@ -281,6 +321,19 @@ func main() {
os.Exit(1)
}

if globalPullSecretKey != nil {
setupLog.Info("creating SecretSyncer controller for watching secret", "Secret", globalPullSecret)
err := (&controllers.PullSecretReconciler{
Client: mgr.GetClient(),
AuthFilePath: authFilePath,
SecretKey: *globalPullSecretKey,
}).SetupWithManager(mgr)
anik120 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "SecretSyncer")
os.Exit(1)
}
}

//+kubebuilder:scaffold:builder

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
Expand All @@ -298,18 +351,8 @@ func main() {
setupLog.Error(err, "problem running manager")
os.Exit(1)
}
tmshort marked this conversation as resolved.
Show resolved Hide resolved
}

func authFilePathIfPresent(logger logr.Logger) string {
_, err := os.Stat(authFilePath)
if os.IsNotExist(err) {
logger.Info("auth file not found, skipping configuration of global auth file", "path", authFilePath)
return ""
}
if err != nil {
logger.Error(err, "unable to access auth file path", "path", authFilePath)
if err := os.Remove(authFilePath); err != nil {
setupLog.Error(err, "failed to cleanup temporary auth file")
os.Exit(1)
}
logger.Info("auth file found, configuring globally for image registry interactions", "path", authFilePath)
return authFilePath
}
110 changes: 110 additions & 0 deletions internal/controllers/pull_secret_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
Copyright 2024.

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 controllers

import (
"context"
"fmt"
"os"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

// PullSecretReconciler reconciles a specific Secret object
// that contains global pull secrets for pulling bundle images
type PullSecretReconciler struct {
client.Client
SecretKey types.NamespacedName
AuthFilePath string
}

func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
if req.Name != r.SecretKey.Name || req.Namespace != r.SecretKey.Namespace {
logger.Error(fmt.Errorf("received unexpected request for Secret %v/%v", req.Namespace, req.Name), "reconciliation error")
return ctrl.Result{}, nil
}

secret := &corev1.Secret{}
err := r.Get(ctx, req.NamespacedName, secret)
if err != nil {
if apierrors.IsNotFound(err) {
logger.Info("secret not found")
return r.deleteSecretFile(logger)
}
logger.Error(err, "failed to get Secret")
return ctrl.Result{}, err
}

return r.writeSecretToFile(logger, secret)
}

// SetupWithManager sets up the controller with the Manager.
func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
_, err := ctrl.NewControllerManagedBy(mgr).
For(&corev1.Secret{}).
WithEventFilter(newSecretPredicate(r.SecretKey)).
Build(r)

return err
}

func newSecretPredicate(key types.NamespacedName) predicate.Predicate {
return predicate.NewPredicateFuncs(func(obj client.Object) bool {
return obj.GetName() == key.Name && obj.GetNamespace() == key.Namespace
})
}

// writeSecretToFile writes the secret data to the specified file
func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secret *corev1.Secret) (ctrl.Result, error) {
// image registry secrets are always stored with the key .dockerconfigjson
// ref: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials
dockerConfigJSON, ok := secret.Data[".dockerconfigjson"]
if !ok {
logger.Error(fmt.Errorf("expected secret.Data key not found"), "expected secret Data to contain key .dockerconfigjson")
return ctrl.Result{}, nil
}
// expected format for auth.json
// https://github.com/containers/image/blob/main/docs/containers-auth.json.5.md
err := os.WriteFile(r.AuthFilePath, dockerConfigJSON, 0600)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to write secret data to file: %w", err)
}
logger.Info("saved global pull secret data locally")
return ctrl.Result{}, nil
}

// deleteSecretFile deletes the auth file if the secret is deleted
func (r *PullSecretReconciler) deleteSecretFile(logger logr.Logger) (ctrl.Result, error) {
logger.Info("deleting local auth file", "file", r.AuthFilePath)
if err := os.Remove(r.AuthFilePath); err != nil {
if os.IsNotExist(err) {
logger.Info("auth file does not exist, nothing to delete")
return ctrl.Result{}, nil
}
return ctrl.Result{}, fmt.Errorf("failed to delete secret file: %w", err)
}
logger.Info("auth file deleted successfully")
return ctrl.Result{}, nil
}
98 changes: 98 additions & 0 deletions internal/controllers/pull_secret_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package controllers_test

import (
"context"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/scheme"
)

func TestSecretSyncerReconciler(t *testing.T) {
secretData := []byte(`{"auths":{"exampleRegistry": "exampledata"}}`)
authFileName := "test-auth.json"
for _, tt := range []struct {
name string
secret *corev1.Secret
addSecret bool
wantErr string
fileShouldExistBefore bool
fileShouldExistAfter bool
}{
{
name: "secret exists, content gets saved to authFile",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "test-secret",
Namespace: "test-secret-namespace",
},
Data: map[string][]byte{
".dockerconfigjson": secretData,
},
},
addSecret: true,
fileShouldExistBefore: false,
fileShouldExistAfter: true,
},
{
name: "secret does not exist, file exists previously, file should get deleted",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "test-secret",
Namespace: "test-secret-namespace",
},
Data: map[string][]byte{
".dockerconfigjson": secretData,
},
},
addSecret: false,
fileShouldExistBefore: true,
fileShouldExistAfter: false,
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
tempAuthFile := filepath.Join(t.TempDir(), authFileName)
clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme)
if tt.addSecret {
clientBuilder = clientBuilder.WithObjects(tt.secret)
}
cl := clientBuilder.Build()

secretKey := types.NamespacedName{Namespace: tt.secret.Namespace, Name: tt.secret.Name}
r := &controllers.PullSecretReconciler{
Client: cl,
SecretKey: secretKey,
AuthFilePath: tempAuthFile,
}
if tt.fileShouldExistBefore {
err := os.WriteFile(tempAuthFile, secretData, 0600)
require.NoError(t, err)
}
res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: secretKey})
if tt.wantErr == "" {
require.NoError(t, err)
} else {
require.ErrorContains(t, err, tt.wantErr)
}
require.Equal(t, ctrl.Result{}, res)

if tt.fileShouldExistAfter {
_, err := os.Stat(tempAuthFile)
require.NoError(t, err)
} else {
_, err := os.Stat(tempAuthFile)
require.True(t, os.IsNotExist(err))
}
})
}
}
Loading