Draft: Filter environments when fetching agent authorizations
Note: This is just a demo MR to facilitate discussion on how to implement #343885 (closed).
Context
We intend to filter the agent authorizations by environment (if applicable) in these 2 places:
-
allowed_agents
API: https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/api/ci/jobs.rb#L251 - CI job KUBECONFIG generation (
GenerateKubeconfigService
): https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/generate_kubeconfig_service.rb
In both places, the agent authorizations are fetched through the AgentAuthorizationsFinder. The allowed_agents
API directly calls the AgentAuthorizationsFinder. In GenerateKubeconfigService
, the agent authorizations is fetched through pipeline.cluster_agent_authorizations
, which uses the AgentAuthorizationsFinder.
Proposed Changes / Points of Discussion
Note: these aren't options. They can be implemented together.
Proposed Change 1
The first change that is done here is to move the AgentAuthorizationsFinder
call away from the Pipeline
model and directly call it in GenerateKubeconfigService
instead.
-
pipeline.cluster_agent_authorizations
isn't called anywhere else - this change caused issues in the
generate_kubeconfig_service_spec
due to the mocks no longer matching how the fetching of agent_authorizations is implemented. Questions:- is the mocking necessary? IE, to make the test faster, or is there another reason for it? Could we test against actual database records like the change introduced in this MR?
- I've had to change the
create
setups in order to make the test pass. Does this match how the actual data should be? (IE, in terms of how the project, pipeline, build, agent, and authorization records are connected)
I don't have enough familiarity with the codebase, so I don't know if I'm missing a reason why we can't make this change.
Proposed Change 2
In this MR, filtering by environment is done through a filter_by_environment
on the AgentAuthorizationsFinder
. (The filtering logic to be added in the actual MR).
An alternative, as discussed with @tigerwnz, is to introduce a service that will be used for filtering the authorizations by environment. So something like (correct me if I misunderstood you, Tiger
# service class
class Clusters::FilterAgentAuthorizationsByEnvironment
def initialize(agent_authorizations, environment)
end
def execute
end
end
# this can then be called as such:
authorizations = ::Clusters::AgentAuthorizationsFinder.new(project).execute
filtered_authorizations = ::Clusters::FilterAgentAuthorizationsByEnvironment.new(authorizations, environment).execute
In either implementation (AgentAuthorizationsFinder.filter_by_environment
method or FilterAgentAuthorizationsByEnvironment
service), the filtering will be done on the GenerateKubeconfigService
(and not on the pipeline.cluster_agent_authorizations
) for the KUBECONFIG generation if we implement Proposed Change 1