terraform-provider-gitlab icon indicating copy to clipboard operation
terraform-provider-gitlab copied to clipboard

RFC: Provide 100% coverage to GitLab API including REST and GraphQL

Open nagyv opened this issue 3 years ago • 30 comments

Problem

As I'm following the issues here for quite some time, and I'm in discussions with several GitLab customers and users around Gitlab - Terraform use cases, often what I see is that we miss various parameters for API endpoints or even support for a whole endpoint. As the API is developed continuously, it seems like an endless work to try to build out support for all parts of the API.

Instead of writing every API endpoint 1 by 1, could we provide a generic solution to every GitLab API?

There are clearly shortcomings of such an approach. As a result, I'd recommend a hybrid solution where we can keep the existing resources, and can add new resources when it seems reasonable, but we can still provide a rough solution for all the other endpoints.

Proposal

The API would have a single spec argument, and its body could be the parameters expected in REST

Example to create/modify a resource

resource "gitlab_rest_api" "example_resource" {
    endpoint = "/projects/${var.project_id}/services/asana"
    spec {
       ...
    }
}

We could provide data resources too, with an optional filter argument.

data "gitlab_rest_api" "example_data" {
    endpoint = "/projects/${var.project_id}/repository/tree"
    filter {}
}

The above would work for GraphQL too.

References

nagyv avatar Feb 03 '21 09:02 nagyv

how would this be different better than using the http provider?

attachmentgenie avatar Feb 03 '21 13:02 attachmentgenie

I saw this same discussion pop up here: https://discuss.hashicorp.com/t/sdk-provider-development-anyone-ever-used-code-generation-or-other-tools-to-simplify-their-provider-development/20301

It sounds like Terraform is not designed for this kind of dynamic model. Building a code generation tool is one way of streamlining development, but that could be a significant effort in itself.

By the way the GitLab REST API has a lot of edge cases where the request/response bodies differ, or there are side effects that the Terraform plugin needs to account for, or some attributes can only be set after initial creation. I haven't tried the GraphQL API personally yet.

armsnyder avatar Feb 04 '21 09:02 armsnyder

@armsnyder

I haven't tried the GraphQL API personally yet.

There are improvements being made to GitLab's GraphQL API. It depends on the situation, but sometimes it's better to use GraphQL instead of the REST API.

Are there any Golang GraphQL libraries you've used in the past, or prefer to use?

Thanks!

btyy77c avatar Feb 10 '21 14:02 btyy77c

@btyy77c No preference from me. Will GitLab possibly invest in maintaining a Golang client SDK for the new GraphQL API? Or is there some spec we can use to generate a client, kind of like Swagger/OpenAPI was for REST?

armsnyder avatar Feb 11 '21 06:02 armsnyder

@nagyv - As a gold customer I concur. Its quite frustrating when new functionality is released, but we cannot use it due to lack of coverage.

There's a lot of core group configuration that we need to control via IaC, but cannot (SSO integration, security/access restrictions, log/security integration etc).

Similarly, when new functionality is released in the UI, but the API itself has to catchup (SAML Group Link being an active example - #503 ).

hammopau avatar Feb 17 '21 08:02 hammopau

@nagyv @armsnyder as a first step and to kinda lay out a roadmap, what would you think is the best way to just "measure" the coverage for now? It doesn't need to be 100% accurate, but just go have a rough idea of what endpoints we support (not even if we support all parameters, let alone behavior).

The OpenAPI spec at this point is pretty much unusable - we could somehow parse https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/api/api.rb or the docs itself. I could do it very dirty real quick, but would more be a one shot thing. What do you think? We gotta start somewhere, somehow ;)

timofurrer avatar Jan 29 '22 11:01 timofurrer

@timofurrer @armsnyder I see a few levels of coverage:

  • GraphQL + REST (GraphQL 0%, REST X%)
  • REST only - @timofurrer my understanding is that your proposal targets this level
  • go-gitlab - as we use go-gitlab for all the REST endpoints, it would be interesting to know what % of the GitLab API is supported by go-gitlab and what percentage is supported by the provider

Today, the maximum we have a chance to support is what go-gitlab already supports. Thus, the most valuable would be to see what % of the go-gitlab features are implemented.

Some related, interesting reading for the weekend: https://gitlab.com/gitlab-org/gitlab/-/issues/348558 - I encourage you to express your opinion :)

nagyv avatar Jan 29 '22 17:01 nagyv

We could get at least a rough measure of our coverage compared to go-gitlab coverage with an analyzer, by checking usage of every struct and struct field in the library (this can be two separate measures). I could work on this.

armsnyder avatar Jan 29 '22 19:01 armsnyder

@armsnyder sounds like a fun and valuable little project, thus go ahead 🍰

@nagyv yes, I was talking about the REST API only for now. To be honest, I haven't explored the GitLab GraphQL API at all.

timofurrer avatar Jan 29 '22 22:01 timofurrer

I haven't tried the GraphQL library yet either. I'm reading up on it now, and it looks like it gives us the ability to auto-discover the API.

@nagyv Do you know if the GraphQL API covers all the v4 REST APIs, or are there missing features?

If it has good coverage then it would an awesome goal for a major release that switches to GraphQL, based around full or partial code generation.

One thing I saw is that GitLab can deprecate parts of the schema and remove them in major releases, so we would need to be careful about our schema versioning. I think we can avoid major provider releases as long as we do that correctly.

armsnyder avatar Feb 02 '22 00:02 armsnyder

@armsnyder My latest information is that neither API covers all the endpoints. There are resources only available over REST or GraphQL. At the same time, GraphQL covers a big number of resources.

Using it is far from trivial. I tried to build a Terraform module to register the GitLab Agent for Kubernetes, and terraform destroy fails, while everything else works. I can not pass the right ID around because of the format used by GraphQL.

I suggest waiting until the API working group figures out the path forward for the GitLab APIs.

nagyv avatar Feb 04 '22 22:02 nagyv

@nagyv With 15.0 approaching there are certain things on the horizon that are changing. For example if we do not attempt to use the GraphQL API in some fashion we lose the capability to configure MR approval rules based off of scan results via a rest API using go-gitlab. With things like this in mind and potentially others not listed in this issue do you have any recommendations going forward for the provider? Has anything come out of the working group that might help? I feel like even if the working group is coming to a consensus it won't be in time to save certain resources before 15.0 hits. Might it be worth doing a gap analysis of the provider of pre vs post 15.0 capabilities?

Shocktrooper avatar Mar 30 '22 03:03 Shocktrooper

I asked the responsible GitLab PM. Hopefully, he'll have an answer.

nagyv avatar Mar 30 '22 09:03 nagyv

@Shocktrooper Based on our discussions in our Working Group so far, consensus continues to point towards GitLab providing deeper GraphQL support as a priority. While we don't have a conclusion, my recommendation is it's worth the investment to begin adding GraphQL support where possible in the Terraform provider. Where there are gaps, please do submit issues or support tickets.

Note there could be some cases where we may continue to only support REST or only support GraphQL, but I believe the most likely case is we'll work towards providing 95%+ of our capabilities in GraphQL and work subsequently to add parity to REST where possible.

As this is currently a moving target, we will share more once we have it.

Regarding deprecations, we do list our deprecations and upcoming removals in our docs site:

  • https://docs.gitlab.com/ee/update/deprecations.html
  • https://docs.gitlab.com/ee/update/removals.html

I can see if I come across other resources to help prepare for %15.0.

Sr. PM, Ecosystem:Integrations, GitLab

granthickman33 avatar Apr 05 '22 19:04 granthickman33

@nagyv @timofurrer @PatrickRice-KSC @armsnyder Any thoughts based on the above information? Also do we want to start pre-creating tickets for 15.0 changes we foresee coming or capture it in some other way.

Shocktrooper avatar Apr 05 '22 20:04 Shocktrooper

I think we likely should open a design issue to figure out how we want to implement GraphQL. I haven't looked to see if there is a good library in golang we could use for that, but I'll take a look at doing so over the next week or so unless someone beats me to it 🙂 .

re: 15.0 braking changes: Yeah, I think it'd be a good idea to get an issue out there that we can use for tracking those changes in the deprecation milestones. I know I've reviewed that list of 14.X deprecation items for my org's usage, but I haven't looked at it yet within the broader scope of the TF provider, so I'd suggest we have one issue where we research that, and spawn other related issues from there.

PatrickRice-KSC avatar Apr 06 '22 03:04 PatrickRice-KSC

Co-opting this issue a bit to mention that I've been looking at GraphQL clients "on the side", and there doesn't seem to be an amazing option for how to proceed here.

Criteria I've been using to try and evaluate clients:

  • The smallest number of dependencies possible to avoid security footprint
  • Type safety of both request and response
  • (Nice to have): Generation of the client based on introspection queries

I've narrowed options down a bit to these three libraries I've been looking at:

  • https://github.com/Khan/genqlient
    • Feels dependency-heavy (7 direct dependencies/ 9 total)
    • Only 13 contributors, with 1 main contributor.
    • Requires us to generate calls up-front, but will generate type safety
  • https://github.com/getoutreach/goql
    • Very nice dependencies (3 direct, 2 of which are very low risk dependencies)
    • follows struct-tag pattern, which is well known, but requires defining whole struct in the tag.
    • Accepts an http.Client behind the scenes, so may be usage with the existing client we have for making calls, leading to a better migration from REST -> GraphQL
    • Very small contributor base who isn't very active
  • https://github.com/hasura/go-graphql-client
    • best dependency footprint (3 direct, all 3 seem very low risk)
    • follows struct-tag pattern, with the tags existing at individual field levels, leading to the cleanest code
    • Accepts an http.Client behind the scenes.
    • Forked from an existing abandoned project, seems close to abandonment itself

So there just.... really aren't very many good options here. I feel like despite the downfalls, genqlient is the best option here, and I would suggest we create a new "graphql" package that we can store the generated clients within. It should be relatively easy to generate individual mutations/queries using the GraphiQL browser from GitLab.

Hopefully someone has some better options or thoughts here!

PatrickRice-KSC avatar Apr 23 '22 03:04 PatrickRice-KSC

There's also the option to not use any third party dependency, and use simple HTTP requests with JSON marshalling/unmarshalling like the GraphQL terraform provider does.

https://github.com/sullivtr/terraform-provider-graphql/blob/master/graphql/query_executor.go

pdecat avatar Apr 23 '22 06:04 pdecat

There's also the option to not use any third party dependency

Absolutely true. I'd say that if we're doing that, we should create a GitLab GraphQL Client project and depend on that from this project (and we may want to do so either way, so we can keep the acceptance tests here clean). The unfortunate issue, though, is that while the GraphQL return values can be parsed from JSON, the query structure is not JSON. Here's an example:

{
  vulnerabilities(projectId:"123456", hasResolution:true, state:DETECTED) {
    pageInfo {
       endCursor,
       startCursor,
       hasNextPage
    }
    nodes {
      id,
      resolvedOnDefaultBranch,
     	title,
      resolvedAt,
      resolvedBy {
        id
      }
    }
  }
}

The values in the parenthesis after vulnerabilities means this is not a valid JSON object, even though the rest of it is :(

That means we either need to build the queries from strings ( 🤮, but doable ) or we need to find a better way, which likely relies on one of the same above options 🙁

PatrickRice-KSC avatar Apr 23 '22 23:04 PatrickRice-KSC

@PatrickRice-KSC It almost feels like we would be rolling our own client library if we didn't do pure strings.

In https://github.com/Khan/genqlient/blob/main/README.md?plain=1#L18 it mentions its used in production for Khan Academy so hopefully there is "some" support behind it in case something is broken in the library

Shocktrooper avatar Apr 24 '22 15:04 Shocktrooper

It almost feels like we would be rolling our own client library if we didn't do pure strings.

I think we'd be rolling our own client regardless of whether we did pure strings or not. There is no other GitLab client library that supports GraphQL, so we're essentially making one to support the provider. If we do strings, we need to handle all the translation of variables, if we use something like genqlient (which I'm completely in support of, btw) we get to skip the "create the request" piece, but we still need to generate all the client for the client which is then still maintained by us.

PatrickRice-KSC avatar Apr 24 '22 21:04 PatrickRice-KSC

I have to hope GitLab will continue to support a REST API. I added a comment in the working group issue comments here.

I checked out https://github.com/Khan/genqlient and found that with it, we still need to write the GraphQL queries and mutations that our provider needs, essentially maintaining our own copy of the API spec. 🙃 The reason is that with GraphQL, the request body has to contain every attribute that you want to get.

Still, if GraphQL becomes a necessity, genqlient is probably the best package for it.

armsnyder avatar Apr 25 '22 00:04 armsnyder

@armsnyder isn't one of the benefits of GraphQL is that you get to read a schema definition and forward generate your mutations that you would execute? Wouldn't that almost translate 1 to 1 with the queries we need?

Shocktrooper avatar Apr 25 '22 01:04 Shocktrooper

@armsnyder isn't one of the benefits of GraphQL is that you get to read a schema definition and forward generate your mutations that you would execute? Wouldn't that almost translate 1 to 1 with the queries we need?

Could you elaborate -- What is forward generating mutations? Are you saying we don't need to hand-write the GraphQL queries and mutations? How does that work?

The overall impression I got from browsing the GitLab GraphQL API is that mutations are not 1:1 with the resource types, so there is going to be added work to model the API as Terraform resources.

Screen Shot 2022-04-24 at 6 37 25 PM

armsnyder avatar Apr 25 '22 01:04 armsnyder

@armsnyder You know what I was wrong about the capabilities of GraphQL I thought the introspection you could do with like a client was more akin to using SOAP with WSDL's and forward class/object generation which is not the case. The closest thing I could find to what I was describing is https://github.com/atulmy/gql-query-builder which comes close to what I thought GraphQL could do but it still seems that you need to define the objects yourself whereas with WSDL's can automatically generate objects and such for you.

Here is an example of WSDL's in java to classes http://javainsimpleway.com/understanding-jax-ws-wsimport-with-example/

Shocktrooper avatar Apr 25 '22 06:04 Shocktrooper

I'm going to start working on a security policy resource using the GQL API here shortly, and I'll ping folks on this thread so we can talk strategy for how to structure the resources!

PatrickRice-KSC avatar May 08 '22 15:05 PatrickRice-KSC

Thanks @PatrickRice-KSC. Is there an issue for that resource yet? There is also this one which needs GraphQL: https://github.com/gitlabhq/terraform-provider-gitlab/issues/707

armsnyder avatar May 09 '22 05:05 armsnyder

@armsnyder - I haven't created one yet, since I'm still trolling around a bit with the right approach. I did notice that issue, but thought it might be good to not start with it given gitlab_project is quite the class, and it would likely be a secondary call after the first call was made. I was hoping for something that would be a better example resource for us moving forward.

PatrickRice-KSC avatar May 09 '22 23:05 PatrickRice-KSC

Marking this issue as stale due to 120 days of inactivity. If this issue receives no comments in the next 30 days it will be closed. Maintainers can also remove the stale label.

Please comment with more information if you would like this issue to remain open.

github-actions[bot] avatar Sep 07 '22 04:09 github-actions[bot]

Removing stale unless we want to let this close and handle in individual resource comments because we have a pattern for GraphQL that's "good enough" for now.

WDYT @timofurrer ?

PatrickRice-KSC avatar Sep 07 '22 12:09 PatrickRice-KSC