-
Notifications
You must be signed in to change notification settings - Fork 94
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.
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
Contract configuration via Rover and Platform API #1421
Comments
reading through this proposal i noticed that you have a proposal on the commands which is great but i also think we're missing a section on which parts of the codebase/files/sections of code you're looking to add more surface area to. have you looked into how the API interacts with Rover? |
@joel-burton thanks for putting this together! This all makes sense to me and fits in with the existing rover design, and I'm happy to pair with anyone taking on implementation here. Taking a step back - I'd love if you could take a minute to look over my proposal here around creating a @caydie-tran re: that last q:
|
Hi yall! Gonna take over the first part of this proposal, namely:
After reading through the specifics outlined here, I'm mostly fine with it. There's a few things though I'd like to change.
|
This all sounds pretty reasonable to me! Few notes:
I don't think we should make these mutually exclusive. Typically in CLIs, you'll see precedence rules like Agree on YAML. Re: variant config. Can you have multiple variants defined in a single config file? Should we be able to? What does the workflow look like for handling multiple contracts across multiple variants? Should they be in separate files and tracked differently in version control? I think if I were to take an opinionated stance here, I would say that we should allow multiple variants in one Part of me really thinks we should figure out "terraform-y" style projects before or as a part of this work, since we are really starting to put some config burden on our users. We have supergraph config, subgraph config, router config, contract config, we want to do things like introduce more lint features and other check types, and even operation collection management. All of this can be really overwhelming and I'm worried that if we continue to just sorta cobble stuff together one by one that we'll leave developers frustrated and that they have to spend time writing their own tooling to integrate with us. |
That sounds fine to me, as long as there's precedent implemented elsewhere I can look at that code and copy whatever pattern they're using. (I imagine it's something like YAML parse, patch, and build, and then submitting that YAML to Studio backend.) If it takes a lot of work though/is non-trivial, it may get scoped for later/follow-up work.
Long-term I think we want to allow this somehow (similar to Kubernetes resource manifests), although I don't think it's in scope to support that for this initiative.
Indeed, if they tried to store configuration documents in version control, they would need to be in separate files.
I think long-term we want folks to be able to store variant configurations in version control to better support config-as-code, and in such a format that they could store multiple YAML configuration documents in one file. I don't think it's in scope for this particular project though. My guess is that when we do introduce such a format, there'll be some YAML key/value pair we can look up to tell that it's using such a format (akin to
I do know long-term there's a lot of interest in supporting "terraform-y" configuration/config-as-code, but I'm not sure when we're planning to take that on in our roadmap/what the product prioritization is. @papollosc might be able to give more insight here. |
Totally understand the sentiment here and we have been calling this config-as-code; however, it would have to make its way to being a "big rock" initiative in order for us to treat it as such. The original goal of this project, configuring contracts via rover, was to solve specific use cases that customers were asking for. I think until config-as-code becomes a big rock which I have no doubt in my mind that it will be but not for H1, we should just build out the original proposal. |
Agree on all fronts here. Avery and I just talked and we came to good interim solution until we can take on config-as-code as a big rock. For the |
That works for me. If we do So then to summarize, the amendments to the proposal are:
|
Makes sense to me! Folks will still be able to script the output of |
This PR implements the first phase of #1421 That is, this PR: - Implements `rover contract describe`, which prints out a human-readable description of the configuration of a contract variant. ``` Describe the configuration of a contract variant from the Apollo graph registry Usage: rover contract describe [OPTIONS] <GRAPH_REF> Arguments: <GRAPH_REF> <NAME>@<VARIANT> of graph in Apollo Studio. @<VARIANT> may be left off, defaulting to @current ``` - Implements `rover contract publish`, which allows creating a contract variant (or updating its configuration) and optionally triggering a launch. ``` Publish an updated contract configuration to the Apollo graph registry and trigger launch in the graph router Usage: rover contract publish [OPTIONS] <--include-tag <INCLUDE_TAG>|--no-include-tags> <--exclude-tag <EXCLUDE_TAG>|--no-exclude-tags> <--hide-unreachable-types|--no-hide-unreachable-types> <GRAPH_REF> Arguments: <GRAPH_REF> <NAME>@<VARIANT> of graph in Apollo Studio. @<VARIANT> may be left off, defaulting to @current Options: --source-variant <SOURCE_VARIANT> The source variant name for this contract variant. Once set, this cannot be changed --include-tag <INCLUDE_TAG> List of tag names to include in the contract schema (e.g. '--include-tag foo --include-tag bar'). To specify an empty list, use --no-include-tags instead --no-include-tags Use an empty include list of tag names for the contract schema. To specify a non-empty list, use --include-tag instead --exclude-tag <EXCLUDE_TAG> List of tag names to exclude from the contract schema (e.g. '--exclude-tag foo --exclude-tag bar'). To specify an empty list, use --no-exclude-tags instead --no-exclude-tags Use an empty exclude list of tag names for the contract schema. To specify a non-empty list, use --exclude-tag instead --hide-unreachable-types Automatically hide types that can never be reached in operations on the contract schema --no-hide-unreachable-types Do not automatically hide types that can never be reached in operations on the contract schema --no-launch Do not trigger a launch in Studio after updating the contract configuration ``` Some notes: - The API for upserting a contract's configuration does not allow specifying a particular key as `null` to indicate "don't change this key's value". This means that most configuration is required to be specified each time the command is run. - The exception is `--source-variant`, which the API allows to be optional. - Follow-up work will take on making these configuration keys nullable in the API. I've set up the Rover code such that making the switch shouldn't be too much work. - Should mainly be changing the `ArgGroup`s to use `required(false)`, updating the `runner.rs`/`types.rs`, and removing the `None` check in `run()`. - Note that instead of saying "not specifying `--include` or `--no-include` means use the pre-existing value", we could also introduce a new argument e.g. `--keep-include` that explicitly keeps the pre-existing value, and add `--keep-include` to the `ArgGroup` instead of making the `ArgGroup` not required. (Similar situation for the other configuration options.) This is something to consider in the follow-up work though/doesn't need to be considered here. Co-authored-by: Caydie Tran <[email protected]> Co-authored-by: Avery Harnish <[email protected]>
# [0.11.0] - 2023-01-24 ## 🚀 Features - **Manage contract configuration - @sachindshinde, #1475 fixes #1421** Rover now includes two commands for creating, modifying, and reading contracts: `rover contract publish` and `rover contract describe`. Further documentation can be found [here](https://www.apollographql.com/docs/rover/commands/contracts). - **Easier file output with new `--output` argument - @gocamille, #1413 fixes #1212** The `--output` argument has long been used to configure the format of Rover's output. i.e. `--output json` configures Rover to print its output as a JSON object. This argument has been _renamed_ to `--format` while maintaining backwards compatibility. `--format json` should be used to configure the format of Rover's output, and the `--output` argument allows you to specify a file to print the output to. Adding `--output schema.graphql` to a `rover subgraph fetch` command will output your schema to a file. `--output data.json --format json` will output the command data to `data.json`. `--output json` will still work by itself but will now print a warning, and `--output [json|plain]` does not work with `--format [json|plain]`. Further documentation can be found [here](https://www.apollographql.com/docs/rover/conventions#output-to-a-file). - **Adds `--router-config` to `rover dev` - @EverlastingBugstopper, #1446 fixes #1373, #1345, and #1468** The new `--router-config` argument for `rover dev` allows you to pass a [router configuration file]() on startup that can configure settings such as header propagaion and CORS policies. Further documentation can be found [here](https://www.apollographql.com/docs/rover/commands/dev/#advanced-configuration). - **Auto-update router versions in `rover dev` - @EverlastingBugstopper, #1432** `rover dev` will automatically use the version of the router specified in [this plugin file](https://github.com/apollographql/rover/blob/main/latest_plugin_versions.json) instead of a hard coded version. ## 🛠 Maintenance - **Better error and help text for ELv2 license argument - @DoumanAsh, #1456 fixes #1455** The help text for the `--elv2-license` argument now includes the expected value for the argument, in addition to the error message returned when the argument is needed but not passed. - **Updates the Ariadne template URL - @patrick91, #1439** - **Updates `./examples/supergraph-demo` to `@apollo/server` v4, and removes `./examples/dev` - @EverlastingBugsttopper, #1442 fixes #1440 and #1441** - **Updates dependencies - @EverlastingBugstopper, #1481, #1450** `apollo-parser` 0.3 -> 0.4 `base64` 0.13 -> 0.21 `git2` 0.15 -> 0.16 `graphql_client` 0.11.0 -> 0.12 `serial_test` 0.9 -> 1.0 `os_info` 3.4 -> 3.5 `os_type` 2.4 -> 2.6 `termcolor` 1.1 -> 1.2 `tokio` 1.21 -> 1.24 ## 📚 Documentation - **Fixes a link to schema check example - @MayCXC, #1431**
Rover x Contracts
Old design brief, from contract ideation
Note
What problem are we solving?
This addition to contracts primarily aims to solve two problems:
Initial proposal is around establishing a new namespace for working with contract variants, starting with commands for creating, updating and reading contract configuration, followed by a second proposal focused on running contract checks via Rover.
For this first proposal we won't be adding the ability to manage contracts via configuration files. There's a couple reasons for this:
There's the definite possibility of adding these in the future, when we have a better understanding of they would be used and managed, but for the initial release it's out of scope.
Contract Configuration
Contract configuration has 3 elements:
@inaccessible
in the supergraph schema)Proposed Commands
contract publish
Analogous to
subgraph publish
, meant to create and update contract variants, and by default launch & publish. However,subgraph publish
pushes out schema updates, where this new command would be pushing out new contract configurations to update contract schemas.Under the hood this command will simply be passing the argument to and calling the
upsertContractVariant()
mutation. This will also require adding this mutation to the platform-apiInput
--source-variant
(optional)upsertContractVariant()
mutation. I think the easiest course of action would be to make that field optional on the mutation as well, instead of adding extra logic to try and populate the value when it's absent.--include
&--exclude
(optional)clap
--no-launch
Output
contract fetch
Similar to other fetch commands,
rover contract fetch my-graph@public-contract
would return the contract configuration to thestdout
output.The main use case here is to be able to easily read & review a given contract's current configuration. This could also be used in a script with
contract publish
to clone a variant.Input
rover contract fetch my-graph@public-contract
Output
Out the the json object returned by the API. Formatted as inputs to the
upsertContractVariant()
mutation.Other Functionality & Surface Area to Consider
contract check
A command to be added in a follow up to allow running checks on a specific contract. Details will be added in a dedicated proposal for this command.
Deleting a Contract
You can delete a contract variant using
rover graph delete
.Fetching Contract Schemas
graph fetch
andsupergraph fetch
should be used for fetching API and Supergraph schemas, respectively.contract list
Should we consider adding a command to list all contract variants based on a given source variant? Not super important for this proposal, but something to consider as we're pushing for feature parity with contract variants.
contract filter
In the future will we need a command analogous to
supergraph compose
for contracts? What workflows would this support? Out of scope for this proposal, but want to mention it as a consideration.The text was updated successfully, but these errors were encountered: