Skip to content

✨ Wire up Service Account#1038

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

Merged
everettraven merged 10 commits into from
Jul 18, 2024
Merged

✨ Wire up Service Account #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d99940d
add logic to return service account
Jul 11, 2024
11d8f90
update permissions and anon token
Jul 11, 2024
dbe5705
updated role yaml
Jul 17, 2024
fbcf4c8
clean up imports
Jul 17, 2024
0e5dc71
update e2e tests
Jul 17, 2024
182648a
fix lint
Jul 17, 2024
09efb2a
extension developer test fixed
Jul 18, 2024
c04d504
stand up sa for upgrade test
Jul 18, 2024
0715393
fixed upgrade test
Jul 18, 2024
d1eb632
linting for extension test
Jul 18, 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
30 changes: 30 additions & 0 deletions cmd/manager/main.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,13 +22,17 @@ import (
"fmt"
"os"
"path/filepath"
"time"

"github.com/spf13/pflag"
"go.uber.org/zap/zapcore"
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
k8slabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/types"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
crcache "sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All@@ -42,6 +46,7 @@ import (

ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/action"
"github.com/operator-framework/operator-controller/internal/authentication"
"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
"github.com/operator-framework/operator-controller/internal/controllers"
Expand DownExpand Up@@ -158,9 +163,34 @@ func main() {
ext := obj.(*ocv1alpha1.ClusterExtension)
return ext.Spec.InstallNamespace, nil
})
coreClient, err := corev1client.NewForConfig(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to create core client")
os.Exit(1)
}
tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))

restConfigMapper := func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
cExt, ok := o.(*ocv1alpha1.ClusterExtension)
if !ok {
return c, nil
}
namespacedName := types.NamespacedName{
Name: cExt.Spec.ServiceAccount.Name,
Namespace: cExt.Spec.InstallNamespace,
}
token, err := tokenGetter.Get(ctx, namespacedName)
if err != nil {
return nil, fmt.Errorf("failed to extract SA token, %w", err)
}
tempConfig := rest.AnonymousClientConfig(c)
tempConfig.BearerToken = token
return tempConfig, nil
}
cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
helmclient.StorageNamespaceMapper(installNamespaceMapper),
helmclient.ClientNamespaceMapper(installNamespaceMapper),
helmclient.RestConfigMapper(restConfigMapper),

@joelanfordjoelanfordJul 16, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this lands soon, all good. But just wanted to let you know that I'm working on making REST config mapping handle storage and client interactions separately. The intent is that we would use our own service account for storage (since that is an implementation detail of our controller) and we would only use the ClusterExtension's service account for the helm client's REST config.

See:

@everettraveneverettravenJul 17, 2024

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.

Thanks for the heads up! I spoke with @theishshah and I think we will focus on getting this in using the soon-to-be-deprecated RestConfigMapper option and as a follow up address this (if the helm-operator-plugins changes land before this gets in)

)
if err != nil {
setupLog.Error(err, "unable to config for creating helm client")
Expand Down
14 changes: 11 additions & 3 deletions config/base/rbac/role.yaml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,11 +5,11 @@ metadata:
name: manager-role
rules:
- apiGroups:
- '*'
- apiextensions.k8s.io
resources:
- '*'
- customresourcedefinitions
verbs:
- '*'
- get
- apiGroups:
- catalogd.operatorframework.io
resources:
Expand All@@ -36,13 +36,21 @@ rules:
- patch
- update
- watch
- apiGroups:
- ""
resources:
- serviceaccounts/token
verbs:
- create
- apiGroups:
- olm.operatorframework.io
resources:
- clusterextensions
verbs:
- get
- list
- patch
- update
- watch
- apiGroups:
- olm.operatorframework.io
Expand Down
2 changes: 1 addition & 1 deletion config/samples/olm_v1alpha1_clusterextension.yaml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ spec:
packageName: argocd-operator
version: 0.6.0
serviceAccount:
name: argocd-installer
name: default
38 changes: 37 additions & 1 deletion hack/test/pre-upgrade-setup.sh
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,6 +33,42 @@ spec:
insecureSkipTLSVerify: true
EOF

kubectl apply -f - <<EOF
apiVersion: v1
kind: ServiceAccount
metadata:
name: upgrade-e2e
namespace: default
EOF

kubectl apply -f - <<EOF
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: upgrade-e2e
rules:
- apiGroups:
- "*"
resources:
- "*"
verbs:
- "*"
EOF

kubectl apply -f - <<EOF
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: upgrade-e2e
subjects:
- kind: ServiceAccount
name: upgrade-e2e
namespace: default
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: upgrade-e2e
EOF

kubectl apply -f - << EOF
apiVersion: olm.operatorframework.io/v1alpha1
Expand All@@ -44,7 +80,7 @@ spec:
packageName: prometheus
version: 1.0.0
serviceAccount:
name: default
name: upgrade-e2e
EOF

kubectl wait --for=condition=Unpacked --timeout=60s ClusterCatalog $TEST_CLUSTER_CATALOG_NAME
Expand Down
5 changes: 3 additions & 2 deletions internal/controllers/clusterextension_controller.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,11 +113,12 @@ type Preflight interface {
Upgrade(context.Context, *release.Release) error
}

//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch;update;patch
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/status,verbs=update;patch
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=create;update;patch;delete;get;list;watch
//+kubebuilder:rbac:groups=*,resources=*,verbs=*
//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get
Comment on lines +120 to +121

@everettraveneverettravenJul 18, 2024

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.

For other reviewers, the serviceaccounts/token permissions were required to get tokens for a provided SA and the customresourcedefinitions permissions are for the CRD Upgrade Safety preflight checks


//+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=clustercatalogs,verbs=list;watch
//+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogmetadata,verbs=list;watch
Expand Down
114 changes: 94 additions & 20 deletions test/e2e/cluster_extension_install_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ import (
"gopkg.in/yaml.v2"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All@@ -38,7 +39,65 @@ const (
var pollDuration = time.Minute
var pollInterval = time.Second

func testInit(t *testing.T) (*ocv1alpha1.ClusterExtension, *catalogd.ClusterCatalog) {
func createServiceAccount(ctx context.Context, name types.NamespacedName) (*corev1.ServiceAccount, error) {
sa := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: name.Name,
Namespace: name.Namespace,
},
}
err := c.Create(ctx, sa)
if err != nil {
return nil, err
}
cr := &rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: name.Name,
},
Rules: []rbacv1.PolicyRule{
{
APIGroups: []string{
"*",
},
Resources: []string{
"*",
},
Verbs: []string{
"*",
},
},
},
}
err = c.Create(ctx, cr)
if err != nil {
return nil, err
}
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: name.Name,
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: name.Name,
Namespace: name.Namespace,
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: name.Name,
},
}
err = c.Create(ctx, crb)
if err != nil {
return nil, err
}

return sa, nil
}

func testInit(t *testing.T) (*ocv1alpha1.ClusterExtension, *catalogd.ClusterCatalog, *corev1.ServiceAccount) {
var err error
extensionCatalog, err := createTestCatalog(context.Background(), testCatalogName, os.Getenv(testCatalogRefEnvVar))
require.NoError(t, err)
Expand All@@ -49,10 +108,18 @@ func testInit(t *testing.T) (*ocv1alpha1.ClusterExtension, *catalogd.ClusterCata
Name: clusterExtensionName,
},
}
return clusterExtension, extensionCatalog

defaultNamespace := types.NamespacedName{
Name: clusterExtensionName,
Namespace: "default",
}

sa, err := createServiceAccount(context.Background(), defaultNamespace)
require.NoError(t, err)
return clusterExtension, extensionCatalog, sa
}

func testCleanup(t *testing.T, cat *catalogd.ClusterCatalog, clusterExtension *ocv1alpha1.ClusterExtension) {
func testCleanup(t *testing.T, cat *catalogd.ClusterCatalog, clusterExtension *ocv1alpha1.ClusterExtension, sa *corev1.ServiceAccount) {
require.NoError(t, c.Delete(context.Background(), cat))
require.Eventually(t, func() bool {
err := c.Get(context.Background(), types.NamespacedName{Name: cat.Name}, &catalogd.ClusterCatalog{})
Expand All@@ -63,21 +130,26 @@ func testCleanup(t *testing.T, cat *catalogd.ClusterCatalog, clusterExtension *o
err := c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, &ocv1alpha1.ClusterExtension{})
return errors.IsNotFound(err)
}, pollDuration, pollInterval)
require.NoError(t, c.Delete(context.Background(), sa))
require.Eventually(t, func() bool {
err := c.Get(context.Background(), types.NamespacedName{Name: sa.Name, Namespace: sa.Namespace}, &corev1.ServiceAccount{})
return errors.IsNotFound(err)
}, pollDuration, pollInterval)
}

func TestClusterExtensionInstallRegistry(t *testing.T) {
t.Log("When a cluster extension is installed from a catalog")
t.Log("When the extension bundle format is registry+v1")

clusterExtension, extensionCatalog := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension)
clusterExtension, extensionCatalog, sa := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

clusterExtension.Spec = ocv1alpha1.ClusterExtensionSpec{
PackageName: "prometheus",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
t.Log("It resolves the specified package with correct bundle path")
Expand DownExpand Up@@ -128,8 +200,8 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
t.Log("When a cluster extension is installed from a catalog")
t.Log("When resolving upgrade edges")

clusterExtension, extensionCatalog := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension)
clusterExtension, extensionCatalog, sa := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

t.Log("By creating an ClusterExtension at a specified version")
Expand All@@ -138,7 +210,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
Version: "1.0.0",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
require.NoError(t, c.Create(context.Background(), clusterExtension))
Expand DownExpand Up@@ -177,8 +249,8 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
t.Log("When a cluster extension is installed from a catalog")
t.Log("When resolving upgrade edges")

clusterExtension, extensionCatalog := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension)
clusterExtension, extensionCatalog, sa := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

t.Log("By creating an ClusterExtension at a specified version")
Expand All@@ -187,7 +259,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
Version: "1.0.0",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
require.NoError(t, c.Create(context.Background(), clusterExtension))
Expand DownExpand Up@@ -225,8 +297,8 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
t.Log("When a cluster extension is installed from a catalog")
t.Log("When resolving upgrade edges")
clusterExtension, extensionCatalog := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension)
clusterExtension, extensionCatalog, sa := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

t.Log("By creating an ClusterExtension at a specified version")
Expand All@@ -235,7 +307,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
Version: "1.0.0",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
require.NoError(t, c.Create(context.Background(), clusterExtension))
Expand DownExpand Up@@ -272,15 +344,15 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
t.Log("When a cluster extension is installed from a catalog")
t.Log("It resolves again when a catalog is patched with new ImageRef")
clusterExtension, extensionCatalog := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension)
clusterExtension, extensionCatalog, sa := testInit(t)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

clusterExtension.Spec = ocv1alpha1.ClusterExtensionSpec{
PackageName: "prometheus",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
t.Log("It resolves the specified package with correct bundle path")
Expand DownExpand Up@@ -351,14 +423,16 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
Name: clusterExtensionName,
},
}
defer testCleanup(t, extensionCatalog, clusterExtension)
sa, err := createServiceAccount(context.Background(), types.NamespacedName{Name: clusterExtensionName, Namespace: "default"})
require.NoError(t, err)
defer testCleanup(t, extensionCatalog, clusterExtension, sa)
defer getArtifactsOutput(t)

clusterExtension.Spec = ocv1alpha1.ClusterExtensionSpec{
PackageName: "prometheus",
InstallNamespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
Name: sa.Name,
},
}
t.Log("It resolves the specified package with correct bundle path")
Expand Down
Loading