Put backend workspaces logic behind feature flag

changed milestone to %15.11

added Category:Remote Development devopscreate featureaddition groupeditor [DEPRECATED] sectiondev typefeature workflowready for development + 1 deleted label

changed the description

This issue made it to workflowready for development without the addition of a frontend or backend tech stack label. Both will be assigned as a result. Bot policy.

added backend frontend labels

@oregand @cwoolley-gitlab Let's discuss the appropriate actor for the feature flag so we can configure it correctly from the start.

Given our current strategy, my understanding is that nothing will prevent Self-managed instances from enabling the flag globally. Question: Does the scope of the flag impact their ability to enable it as well? So if we scope it to user and Acme Inc. enables it, could they pass in specific users on their end?

@ericschurter

Given our current strategy, my understanding is that nothing will prevent Self-managed instances from enabling the flag globally. Question: Does the scope of the flag impact their ability to enable it as well? So if we scope it to user and Acme Inc. enables it, could they pass in specific users on their end?

Correct.

My suggestion would be we use Group level actors, WDYT @cwoolley-gitlab ?

Oh right, that's new after we did the Web IDE roll out, right?

I agree, if we can pass in a group instead of individual users, that is ideal.

I'd prefer not to do it by project because it could get confusing for someone who thinks they have access to the feature... navigating to a project and not remembering whether it's enabled or disabled.

I'd prefer not to do it by project because it could get confusing for someone who thinks they have access to the feature... navigating to a project and not remembering whether it's enabled or disabled.

Its a new development we can lean on: https://docs.gitlab.com/ee/development/feature_flags/#feature-groups

Its a new development we can lean on: https://docs.gitlab.com/ee/development/feature_flags/#feature-groups

I think that's something different 😄

But we can still use a group as an actor, as documented here:

https://docs.gitlab.com/ee/development/feature_flags/#feature-actors

The only "gotcha" is this:

The same actor type must be used consistently for all invocations of Feature.enabled?

So if we pick group as our actor, that means we can't use the same flag to enable it on a per-user or per-project basis.

I think this was the limitation we ran into with the Web IDE too.

I also think that's probably part of what https://docs.gitlab.com/ee/development/feature_flags/#feature-groups was intended to help address, but it seems like that's hardcoded to just allow all gitlab_team_members but we couldn't use it as is for anything other than that, unless we implemented our own custom group.

At least this is how I understand it all 😄

@ericschurter @oregand @vtak Now that I have started implementing this, I realize there's some problems with using the group actor.

The problems arise because we've now moved the feature to the My Work level in the menu instead of under a Group context.

This means that we can't use a group-scoped feature flag to disable the overall UI rendering and Workspace Create form under "My Work", because there's no concept of a "group" to scope it to at that level.

But, we have options.

First, lets remember that there's two separate goals we could want from our feature flag(s) for this feature:

To prevent users from using the feature if it is not enabled for a specific top-level group.
As a global "kill switch", which can completely disable the feature in the event of unexpected problems.

Option 1:

If we are OK with the feature being enabled globally for all users, on both SaaS and self-managed instances, we can go with a single, non-scoped feature flag that achieves goal number 2.

This is the easiest approach, but we have to be OK with enabling or disabling it globally.

Option 2:

If we still want to be able to enable-disable it on a per-root-group basis, then we can still have one "global" flag which achieves goal number 2, and then have an additional "per-root-group" flag which is required to enable it on a per-root-group basis.

However, there's implications of this approach:

It's a more complex approach (and these flags are already complex to reason about, implement and test, and trying to test two flags in conjunction makes it even more so). Plus, every flag is supposed to have a dedicated rollout issue and be managed over its lifetime (we get reports for every existing feature flag for our group until they are deleted), so adding a second one adds even more overhead.
Both flags have to be enabled in conjunction as appropriate. E.g. the global flag will enable the UI and kubernetes API, and the 'per-root-group' flag will control the actual display of the workspaces as returned from the GraphQL API to be displayed in the UI, such as for the workspace list, or starting/restarting/stopping/terminating a workspace.
If you want to allow users to create workspaces in multiple root groups, then the per-root-group flag should be consistently enabled/disabled for all root groups at the same time, or else they would see nothing in the list UI (and currently it's just a generic error). This is because it would be even more complex at this point to try to selectively apply the per-group flag to individual workstation retrieval from the GraphQL - the flag checks are done at a higher level in the logic, and also there's even more implications of this about potentially orphaning some of the user's workspaces.
All of this has to be documented and explained to on-prem admins.
Again, as you can see, it's more complex and harder to reason about.

So, if it's acceptable, I'm in favor of Option 1 - make it a global per-instance flag.

But if that isn't acceptable, we need to have some discussions about the implications of this and make sure they are fully understood.

@ericschurter @oregand

This means that we can't use a group-scoped feature flag to disable the overall UI rendering and Workspace Create form under "My Work", because there's no concept of a "group" to scope it to at that level.

Is it acceptable to still have a group-level FF like we originally planned but let the UI be out of scope of the feature flag. For me, the feature flag is a protective switch to save GitLab in case of a disaster.

This will also take care of the following point Chad raised above -

If you want to allow users to create workspaces in multiple root groups, then the per-root-group flag should be consistently enabled/disabled for all root groups at the same time, or else they would see nothing in the list UI (and currently it's just a generic error). This is because it would be even more complex at this point to try to selectively apply the per-group flag to individual workstation retrieval from the GraphQL - the flag checks are done at a higher level in the logic, and also there's even more implications of this about potentially orphaning some of the user's workspaces.

So

The UI ("My Workspaces" sidebar entry, workspace creation form, workspace listing page) will be visible to everyone
When a user tries to create a project(whose root group has FF enabled), backend will allow it.
When a user tries to create a project(whose root group has FF disabled), backend will NOT allow it and the UI can show an error that this root-group does not have the FF enabled yet.

WDYT @cwoolley-gitlab ?

Is it acceptable to still have a group-level FF like we originally planned but let the UI be out of scope of the feature flag. For me, the feature flag is a protective switch to save GitLab in case of a disaster.

No, this doesn't really help much. Because it's not just the UI, it's the kubernetes REST API that also has no group scope (because it can send requests applying to any group in the instance). We'd still have to have a separate global flag for it.

Plus, this still has all the same complexity as the second case, where we have to reason about coordinating flag enabling for overlapping group cases when an admin wants to enable the flag for multiple groups.

Finally, this would still result in a broken and unusable UI being presented to the user if the flag is disabled, and we have already had reviewers questioning that in the reviewers channel.

Also, I forgot to mention another drawback of the per-root-group flag appraoch: the implementation of the feature flag logic itself could potentially cause performance problems.

This is because finding the root ancestor can be an expensive operation, especially in groups with large hierarchies and many subgroups.

And we have to perform this check for every workspace the user retrieves to list, because each workstations project could potentially be in any different root group.

@cwoolley-gitlab @vtak There are so many complications for Option 2 (per-root-group).

I'm fine with Option 1, let's make this a global/instance level setting and just be done with it.

I'm fine with Option 1, let's make this a global/instance level setting and just be done with it.

What Eric said 👍

mentioned in issue gitlab-org/quality/triage-reports#11912 (closed)

added Deliverable label

removed 1 deleted label

removed frontend label

@cwoolley-gitlab - Is this a duplicate of #403621 (closed) ?

/cc @hkhanna2

Yes, I'll close the other one.

changed the description

marked #403621 (closed) as a duplicate of this issue

marked this issue as related to #403621 (closed)

changed epic to &9893 (closed)

mentioned in issue #403621 (closed)

This issue is scheduled for completion in this milestone but doesn't have an assignee. Changing health status to 'needs attention'.

Issue participants are welcome to override this by setting the health status to another value.

changed health status to needs attention

assigned to @cwoolley-gitlab

@cwoolley-gitlab This issue looks like it may slip this current milestone. Can you leave a 👍 or 👎 to signify if you are on track to deliver this issue? Please also consider updating the issue's Health Status or Milestone to reflect its current state, and communicate with your Product Manager as appropriate.

Bot policy.

/cc @ericschurter

mentioned in issue gitlab-org/quality/triage-reports#12004 (closed)

This issue is scheduled for completion in this milestone but is in an early development stage. Changing health status to 'at risk'.

Issue participants are welcome to override this by setting the health status to another value.

changed health status to at risk

added quad-planningcomplete-no-action label

removed quad-planningcomplete-no-action label

set weight to 2

added groupide label and removed groupeditor [DEPRECATED] label

changed milestone to %16.0

added missed-deliverable missed:15.11 labels

Setting health status to on track as the milestone has just begun.

Issue participants are welcome to override this by setting the health status to another value.

changed health status to on track

mentioned in issue gitlab-org/quality/triage-reports#12166 (closed)

changed the description

@shekharpatnaik and @vtak - Can you review the task list I've added in the description here and make sure we have all endpoints covered, especially any ones related to auth?

cc @splattael

For workspaces proxy, we don't really have feature flags since its outside the rails monolith. However, customers always have the option to uninstall it using helm uninstall if they don't want to use it.

@shekharpatnaik My main question is whether there's some other rails endpoint besides the ones I've listed.

I'm unclear on what the exact rails endpoints related to this are, if any, based on @vtak 's comment here in slack: https://gitlab.slack.com/archives/C052T8Z39GA/p1682595259743179?thread_ts=1682567314.997409&cid=C052T8Z39GA

For authentication - I think we are using an existing GraphQL endpoint - https://gitlab.com/gitlab-org/remote-development/gitlab-workspaces-proxy/-/blob/main/pkg/gitlab/user.go#L15
For authorization, we use a new GraphQL endpoint - https://gitlab.com/gitlab-org/remote-development/gitlab-workspaces-proxy/-/blob/main/pkg/gitlab/workspace.go#L17
So authentication should work and authorization should fail because that endpoint would be behind feature flag AFAICT. 
@Shekhar
 , please correct me if I'm wrong here.

Could you help update the task list in the description here with any additional endpoints other than the ones I've listed?

@cwoolley-gitlab

internal/kubernetes/authorize_proxy_user - I'm not sure what new thing we've added here? Maybe you meant kubernetes/agent_configuration ? In which case, I think there is little need to put this behind a FF. It is merely updating the DB and then doing normal GA4K thing(sending agent configuration to agent). But if this helps with the reviews, by all means.

The create, update, reconcile endpoints cover most of the exposure points. 👍

Re: Others? Authorization endpoints?

@shekharpatnaik - It looks like we are using the following GraphQL endpoint for authorization in the gitlab-workspaces-proxy

	gid := fmt.Sprintf("gid://gitlab/RemoteDevelopment::Workspace/%s", workspaceID)
	var query struct {
		Workspace *Workspace `graphql:"workspace(id: $workspaceID)"`
	}

Can we put this behind feature flag? I must admit, I know little about GraphQL so I might come off as ignorant over here. But bear with me.

@cwoolley-gitlab ah I see. Its the workspace(id: $Id) endpoint at ee/app/graphql/ee/types/query_type.rb that we use. We compare the user Id returned from that endpoint with the logged in user.

Right, the fields on the root QueryType. Thanks Shekar, I've added those to the list. I've asked a question in slack about how to check the flag for this one:

If I have the following field on the root EE QueryType:

        field :workspace, ::Types::RemoteDevelopment::WorkspaceType,
              null: true,
              description: 'Find a workspace.' do
                argument :id, ::Types::GlobalIDType[::RemoteDevelopment::Workspace],
                required: true,
                description: 'Find a workspace by its ID.'

…how would I put a feature flag check on it to disable its use if the flag is disabled? Is there a generic ID resolver that gets used that I can somehow add to?

@cwoolley-gitlab @shekharpatnaik

I don't know how but can we also somehow put the authentication endpoint that is used in gitlab-workspaces-proxy behind a feature flag?

That way, we have total control over everything.

Although, I doubt if it is possible because I think we are using a generic GraphQL endpoint for authentication 🤔

WDYT?

I doubt if it is possible because I think we are using a generic GraphQL endpoint for authentication

Right, we can't do it there. But I got the answer in the slack thread:

Something like https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#feature-flagged-field?

So we just need to put the feature flag check in a custom method for the root QueryType.workspace field.

I've updated the description to indicate this.

changed the description

changed iteration to Workspaces Apr 24, 2023 - Apr 30, 2023

changed the description

mentioned in issue #391543 (closed)

changed the description

removed missed-deliverable label

removed missed:15.11 label

mentioned in merge request !119152 (merged)

changed the description

removed iteration Workspaces Apr 24, 2023 - Apr 30, 2023

changed iteration to Workspaces May 1, 2023 - May 7, 2023

mentioned in issue gitlab-org/quality/triage-reports#12229 (closed)

changed the description

closed

mentioned in issue #409038 (closed)

added quad-planningcomplete-no-action label

marked the checklist item Add guards to all endpoints as completed

marked the checklist item GraphQL Workspace create as completed

marked the checklist item GraphQL Workspace update as completed

marked the checklist item GraphQL root QueryType workspace by id field (see https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#feature-flagged-field) as completed

marked the checklist item GraphQL root QueryType workspaces by ids field as completed

marked the checklist item GraphQL User interface workspaces field as completed

marked the checklist item REST internal kubernetes workspace reconcile: internal/kubernetes/modules/remote_development/reconcile as completed

marked the checklist item REST internal kubernetes agent config (existing endpoint that is used by other GA4K features, but we should put guard around our part of the request logic): internal/kubernetes/authorize_proxy_user as completed

marked the checklist item Update documentation with references on how to enable feature flags. as completed

added rd-workflowdone label

Put backend workspaces logic behind feature flag

Overview

Description

Tasks

Designs

Child items 0

Activity

Put backend workspaces logic behind feature flag

Overview

Description

Tasks

Relates to

Activity