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

ASC-402: Copy the SA token of sub-spaces only for specific tiers #902

Closed
wants to merge 9 commits into from
Closed

ASC-402: Copy the SA token of sub-spaces only for specific tiers #902

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
de3c499
Copy the SA token of sub-spaces only for specific tiers
Kartikey-star Oct 20, 2023
47aca72
Copy the SA token of sub-spaces only for specific tiers
Kartikey-star Oct 25, 2023
6117ff1
Merge branch 'master' into sa_token_copy
Kartikey-star Oct 25, 2023
973f674
Merge branch 'master' into sa_token_copy
Kartikey-star Oct 26, 2023
34e49a8
Merge branch 'master' into sa_token_copy
mfrancisc Oct 30, 2023
2e57912
Merge branch 'master' into sa_token_copy
mfrancisc Nov 6, 2023
6651095
Merge branch 'master' into sa_token_copy
mfrancisc Nov 9, 2023
c0aa111
Merge branch 'master' into sa_token_copy
mfrancisc Nov 10, 2023
0cd9550
Merge branch 'master' into sa_token_copy
mfrancisc Nov 13, 2023
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
52 changes: 27 additions & 25 deletions controllers/spacerequest/spacerequest_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, err
}

subSpace, createdOrUpdated, err := r.ensureSpace(ctx, memberClusterWithSpaceRequest, spaceRequest)
subSpace, tier, createdOrUpdated, err := r.ensureSpace(ctx, memberClusterWithSpaceRequest, spaceRequest)
// if there was an error or if subSpace was just created or updated,
// let's just return.
if err != nil || createdOrUpdated {
return ctrl.Result{}, err
}

// ensure there is a secret that provides admin access to each provisioned namespaces of the subSpace
if err := r.ensureSecretForProvisionedNamespaces(ctx, memberClusterWithSpaceRequest, spaceRequest, subSpace); err != nil {
return reconcile.Result{}, r.setStatusFailedToCreateSubSpace(ctx, memberClusterWithSpaceRequest, spaceRequest, err)
if tier.Spec.SpaceRequestConfig != nil {
// ensure there is a secret that provides admin access to each provisioned namespaces of the subSpace
if err := r.ensureSecretForProvisionedNamespaces(ctx, memberClusterWithSpaceRequest, spaceRequest, subSpace, tier.Spec.SpaceRequestConfig.ServiceAccountName); err != nil {
return reconcile.Result{}, r.setStatusFailedToCreateSubSpace(ctx, memberClusterWithSpaceRequest, spaceRequest, err)
}
mfrancisc marked this conversation as resolved.
Show resolved Hide resolved
}

// update spaceRequest conditions and target cluster url
err = r.updateSpaceRequest(ctx, memberClusterWithSpaceRequest, spaceRequest, subSpace)

Expand Down Expand Up @@ -160,25 +161,26 @@ func (r *Reconciler) addFinalizer(ctx context.Context, memberCluster cluster.Clu
return nil
}

func (r *Reconciler) ensureSpace(ctx context.Context, memberCluster cluster.Cluster, spaceRequest *toolchainv1alpha1.SpaceRequest) (*toolchainv1alpha1.Space, bool, error) {
func (r *Reconciler) ensureSpace(ctx context.Context, memberCluster cluster.Cluster, spaceRequest *toolchainv1alpha1.SpaceRequest) (*toolchainv1alpha1.Space, *toolchainv1alpha1.NSTemplateTier, bool, error) {
logger := log.FromContext(ctx)
logger.Info("ensuring subSpace")

// find parent space from namespace labels
parentSpace, err := r.getParentSpace(ctx, memberCluster, spaceRequest)
if err != nil {
return nil, false, err
return nil, nil, false, err
}
// parentSpace is being deleted
if util.IsBeingDeleted(parentSpace) {
return nil, false, errs.New("parentSpace is being deleted")
return nil, nil, false, errs.New("parentSpace is being deleted")
}

var tier toolchainv1alpha1.NSTemplateTier

// validate tierName
if err := r.validateNSTemplateTier(ctx, spaceRequest.Spec.TierName); err != nil {
return nil, false, err
if tier, err = r.validateNSTemplateTier(ctx, spaceRequest.Spec.TierName); err != nil {
return nil, nil, false, err
}

// create if not found on the expected target cluster
subSpace := &toolchainv1alpha1.Space{}
if err := r.Client.Get(ctx, types.NamespacedName{
Expand All @@ -189,21 +191,21 @@ func (r *Reconciler) ensureSpace(ctx context.Context, memberCluster cluster.Clus
// no spaces found, let's create it
logger.Info("creating subSpace")
if err := r.setStatusProvisioning(ctx, memberCluster, spaceRequest); err != nil {
return nil, false, errs.Wrap(err, "error updating status")
return nil, nil, false, errs.Wrap(err, "error updating status")
}
subSpace, err = r.createNewSubSpace(ctx, spaceRequest, parentSpace)
if err != nil {
// failed to create subSpace
return nil, false, r.setStatusFailedToCreateSubSpace(ctx, memberCluster, spaceRequest, err)
return nil, nil, false, r.setStatusFailedToCreateSubSpace(ctx, memberCluster, spaceRequest, err)
}
return subSpace, true, nil // a subSpace was created
return subSpace, &tier, true, nil // a subSpace was created
}
// failed to create subSpace
return nil, false, r.setStatusFailedToCreateSubSpace(ctx, memberCluster, spaceRequest, err)
return nil, nil, false, r.setStatusFailedToCreateSubSpace(ctx, memberCluster, spaceRequest, err)
}
logger.Info("subSpace already exists")
updated, err := r.updateExistingSubSpace(ctx, spaceRequest, subSpace)
return subSpace, updated, err
return subSpace, &tier, updated, err
}

func (r *Reconciler) createNewSubSpace(ctx context.Context, spaceRequest *toolchainv1alpha1.SpaceRequest, parentSpace *toolchainv1alpha1.Space) (*toolchainv1alpha1.Space, error) {
Expand Down Expand Up @@ -237,9 +239,9 @@ func (r *Reconciler) updateExistingSubSpace(ctx context.Context, spaceRequest *t
}

// validateNSTemplateTier checks if the provided tierName in the spaceRequest exists and is valid
func (r *Reconciler) validateNSTemplateTier(ctx context.Context, tierName string) error {
func (r *Reconciler) validateNSTemplateTier(ctx context.Context, tierName string) (toolchainv1alpha1.NSTemplateTier, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

how about returning a pointer? This way you could return tier or nil.

Suggested change
func (r *Reconciler) validateNSTemplateTier(ctx context.Context, tierName string) (toolchainv1alpha1.NSTemplateTier, error) {
func (r *Reconciler) validateNSTemplateTier(ctx context.Context, tierName string) (*toolchainv1alpha1.NSTemplateTier, error) {

if tierName == "" {
return fmt.Errorf("tierName cannot be blank")
return toolchainv1alpha1.NSTemplateTier{}, fmt.Errorf("tierName cannot be blank")
}
// check if requested tier exists
tier := &toolchainv1alpha1.NSTemplateTier{}
Expand All @@ -248,12 +250,12 @@ func (r *Reconciler) validateNSTemplateTier(ctx context.Context, tierName string
Name: tierName,
}, tier); err != nil {
if errors.IsNotFound(err) {
return err
return *tier, err
}
// Error reading the object - requeue the request.
return errs.Wrap(err, "unable to get the current NSTemplateTier")
return *tier, errs.Wrap(err, "unable to get the current NSTemplateTier")
}
return nil
return *tier, nil
}

// updateSubSpace updates the tierName and targetClusterRoles from the spaceRequest to the subSpace object
Expand Down Expand Up @@ -397,7 +399,7 @@ func (r *Reconciler) deleteExistingSubSpace(ctx context.Context, subSpace *toolc
return true, nil
}

func (r *Reconciler) ensureSecretForProvisionedNamespaces(ctx context.Context, memberClusterWithSpaceRequest cluster.Cluster, spaceRequest *toolchainv1alpha1.SpaceRequest, subSpace *toolchainv1alpha1.Space) error {
func (r *Reconciler) ensureSecretForProvisionedNamespaces(ctx context.Context, memberClusterWithSpaceRequest cluster.Cluster, spaceRequest *toolchainv1alpha1.SpaceRequest, subSpace *toolchainv1alpha1.Space, serviceAccountName string) error {
logger := log.FromContext(ctx)

if len(subSpace.Status.ProvisionedNamespaces) == 0 {
Expand Down Expand Up @@ -427,7 +429,7 @@ func (r *Reconciler) ensureSecretForProvisionedNamespaces(ctx context.Context, m
switch {
case len(secretList.Items) == 0:
// create the secret for this namespace
clientConfig, err := r.generateKubeConfig(subSpaceTargetCluster, namespace.Name)
clientConfig, err := r.generateKubeConfig(subSpaceTargetCluster, namespace.Name, serviceAccountName)
if err != nil {
return err
}
Expand Down Expand Up @@ -483,11 +485,11 @@ func (r *Reconciler) ensureSecretForProvisionedNamespaces(ctx context.Context, m
return nil
}

func (r *Reconciler) generateKubeConfig(subSpaceTargetCluster cluster.Cluster, namespace string) (*api.Config, error) {
func (r *Reconciler) generateKubeConfig(subSpaceTargetCluster cluster.Cluster, namespace, serviceAccountName string) (*api.Config, error) {
// create a token request for the admin service account
token, err := restclient.CreateTokenRequest(subSpaceTargetCluster.RESTClient, types.NamespacedName{
Namespace: namespace,
Name: toolchainv1alpha1.AdminServiceAccountName,
Name: serviceAccountName,
Copy link
Contributor

Choose a reason for hiding this comment

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

I see there are few other occurrences to be replaced, line 508 and 511. Could you please replace those as well?

}, TokenRequestExpirationSeconds)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion deploy/templates/nstemplatetiers/appstudio-env/ns_env.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ objects:
kind: ServiceAccount
metadata:
namespace: ${SPACE_NAME}-env
name: namespace-manager
name: ${SERVICE_ACCOUNT_NAME}
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it is fine to hard code the name here and in the tier.yaml. Also because unless I'm missing something, you are not populating this variable SERVICE_ACCOUNT_NAME right now.


- apiVersion: rbac.authorization.k8s.io/v1
kind: Role
Expand Down
3 changes: 3 additions & 0 deletions deploy/templates/nstemplatetiers/appstudio-env/tier.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ objects:
templateRef: ${MAINTAINER_TEMPL_REF}
contributor:
templateRef: ${CONTRIBUTOR_TEMPL_REF}
spaceRequestConfig:
serviceAccountName: ${SERVICE_ACCOUNT_NAME}
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment above, we could hard code the name here I guess.

parameters:
- name: NAMESPACE
- name: CLUSTER_TEMPL_REF
- name: ENV_TEMPL_REF
- name: ADMIN_TEMPL_REF
- name: MAINTAINER_TEMPL_REF
- name: CONTRIBUTOR_TEMPL_REF
- name: SERVICE_ACCOUNT_NAME
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module github.com/codeready-toolchain/host-operator

require (
github.com/codeready-toolchain/api v0.0.0-20230918195153-739e8fb09a33
github.com/codeready-toolchain/api v0.0.0-20231018174322-a2004884afce
github.com/codeready-toolchain/toolchain-common v0.0.0-20231017151548-4fd4e48ab6b7
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/ghodss/yaml v1.0.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoC
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
github.com/codeready-toolchain/api v0.0.0-20230918195153-739e8fb09a33 h1:hxXfcFq2JgFISVxrkISg8m9DZMzpcPWRjPspx3M3Sxo=
github.com/codeready-toolchain/api v0.0.0-20230918195153-739e8fb09a33/go.mod h1:nn3W6eKb9PFIVwSwZW7wDeLACMBOwAV+4kddGuN+ARM=
github.com/codeready-toolchain/api v0.0.0-20231018174322-a2004884afce h1:QWo6mVpEvyeeu1RF4qY+l56s28BFgihr2qQskgCev4A=
github.com/codeready-toolchain/api v0.0.0-20231018174322-a2004884afce/go.mod h1:bImSKnxrpNmCmW/YEGiiZnZqJm3kAmfP5hW4YndK0hE=
github.com/codeready-toolchain/toolchain-common v0.0.0-20231017151548-4fd4e48ab6b7 h1:zCHPxkQbOFqONzKeQQNiAM6e0oNz1WfZFcFzDuuvxGs=
github.com/codeready-toolchain/toolchain-common v0.0.0-20231017151548-4fd4e48ab6b7/go.mod h1:SnZewh0DLwAELKLsW+R6NKaKmiRBuMI1iMYSkfyZG6A=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
Expand Down
Loading