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.

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

Deprecate GraphQL #10355

Merged
merged 14 commits into from
Nov 15, 2024
Merged

Deprecate GraphQL #10355

merged 14 commits into from
Nov 15, 2024

Conversation

sheidkamp
Copy link

Description

Deprecate GraphQL - Mark as deprecated in API and the create changelog entry

API changes

Add deprecation warnings

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@sheidkamp sheidkamp requested a review from a team as a code owner November 15, 2024 01:37
@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/7159

@@ -287,7 +287,7 @@ message Settings {

message FdsOptions {
// Enable function discovery service on GraphQL gRPC and OpenApi upstreams. Defaults to true.
Copy link

Choose a reason for hiding this comment

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

we should also add language about deprecation to the field comments; i don't think the [deprecated = true] tag has any effect on our generated docs.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, updated text

}

// GraphQL settings used by the control plane and UI.
// Deprecated: GraphQL settings used by the control plane and UI.
message GraphqlOptions {
Copy link

Choose a reason for hiding this comment

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

i think there are some other files that we can add the deprecation messages to.

these are all the files i see 'graphql' references in. we probably don't need to modify the external/envoy ones since those are just imported into the user-facing protos

projects/gateway/api/v1/virtual_service.proto
projects/gloo/api/external/envoy/extensions/graphql/graphql.proto
projects/gloo/api/external/envoy/extensions/graphql/stitching.proto
projects/gloo/api/v1/enterprise/options/graphql/v1beta1/graphql.proto
projects/gloo/api/v1/options/graphql/graphql.proto
projects/gloo/api/v1/options/service_spec.proto
projects/gloo/api/v1/proxy.proto
projects/gloo/api/v1/settings.proto

Copy link
Author

Choose a reason for hiding this comment

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

TY, I think something has been off with my VS code search. Updated in a bunch of places. Agree that we don't need to update the external/envoy ones as those docs are going to get deleted as part of an in-flight ticket.

Copy link
Author

@sheidkamp sheidkamp left a comment

Choose a reason for hiding this comment

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

UPdated, ty!

@@ -287,7 +287,7 @@ message Settings {

message FdsOptions {
// Enable function discovery service on GraphQL gRPC and OpenApi upstreams. Defaults to true.
Copy link
Author

Choose a reason for hiding this comment

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

Good catch, updated text

}

// GraphQL settings used by the control plane and UI.
// Deprecated: GraphQL settings used by the control plane and UI.
message GraphqlOptions {
Copy link
Author

Choose a reason for hiding this comment

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

TY, I think something has been off with my VS code search. Updated in a bunch of places. Agree that we don't need to update the external/envoy ones as those docs are going to get deleted as part of an in-flight ticket.

@@ -213,7 +213,7 @@ top-level `RouteTable` resources.
| `redirectAction` | [.gloo.solo.io.RedirectAction](../../../../gloo/api/v1/proxy.proto.sk/#redirectaction) | Redirect actions tell the proxy to return a redirect response to the downstream client. Only one of `redirectAction`, `routeAction`, `directResponseAction`, `delegateAction`, or `graphqlApiRef` can be set. |
| `directResponseAction` | [.gloo.solo.io.DirectResponseAction](../../../../gloo/api/v1/proxy.proto.sk/#directresponseaction) | Return an arbitrary HTTP response directly, without proxying. Only one of `directResponseAction`, `routeAction`, `redirectAction`, `delegateAction`, or `graphqlApiRef` can be set. |
| `delegateAction` | [.gateway.solo.io.DelegateAction](../virtual_service.proto.sk/#delegateaction) | Delegate routing actions for the given matcher to one or more RouteTables. Only one of `delegateAction`, `routeAction`, `redirectAction`, `directResponseAction`, or `graphqlApiRef` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Enterprise-Only: THIS FEATURE IS IN TECH PREVIEW. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, `directResponseAction`, or `delegateAction` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, `directResponseAction`, or `delegateAction` can be set. |

Choose a reason for hiding this comment

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

Suggested change
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, `directResponseAction`, or `delegateAction` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, `directResponseAction`, or `delegateAction` can be set. |

@@ -46,6 +46,7 @@ weight: 5
### RequestTemplate


Deprecated: The GraphQL feature of Gloo Gateway will be removed in a future release

Choose a reason for hiding this comment

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

Suggested change
Deprecated: The GraphQL feature of Gloo Gateway will be removed in a future release
Deprecated: The GraphQL feature of Gloo Gateway will be removed in a future release.

Choose a reason for hiding this comment

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

missing period applies to all of these notes

@@ -431,7 +431,7 @@ Routes declare the entry points on virtual hosts and the action to take for matc
| `routeAction` | [.gloo.solo.io.RouteAction](../proxy.proto.sk/#routeaction) | This action is the primary action to be selected for most routes. The RouteAction tells the proxy to route requests to an upstream. Only one of `routeAction`, `redirectAction`, `directResponseAction`, or `graphqlApiRef` can be set. |
| `redirectAction` | [.gloo.solo.io.RedirectAction](../proxy.proto.sk/#redirectaction) | Redirect actions tell the proxy to return a redirect response to the downstream client. Only one of `redirectAction`, `routeAction`, `directResponseAction`, or `graphqlApiRef` can be set. |
| `directResponseAction` | [.gloo.solo.io.DirectResponseAction](../proxy.proto.sk/#directresponseaction) | Return an arbitrary HTTP response directly, without proxying. Only one of `directResponseAction`, `routeAction`, `redirectAction`, or `graphqlApiRef` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Enterprise-Only: THIS FEATURE IS IN TECH PREVIEW. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, or `directResponseAction` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: This feature is deprecated and wil. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, or `directResponseAction` can be set. |

Choose a reason for hiding this comment

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

Suggested change
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: This feature is deprecated and wil. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, or `directResponseAction` can be set. |
| `graphqlApiRef` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Deprecated, Enterprise-Only: THIS FEATURE IS DEPRECATED AND WILL BE REMOVED IN A FUTURE RELEASE. APIs are versioned as alpha and subject to change. A reference to a GraphQLApi CR. Resolution of the client request to upstream(s) will be delegated to the resolution policies defined in the GraphQLApi CR. If configured, the graphql filter will operate instead of the envoy router filter, so configuration (such as retries) that applies to the router filter will not be applied. Only one of `graphqlApiRef`, `routeAction`, `redirectAction`, or `directResponseAction` can be set. |

@@ -957,6 +957,7 @@ options for configuring admission control / validation
### ConsoleOptions


Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
Deprecated. The GraphQL feature will be removed in a future release.
Deprecated: The GraphQL feature will be removed in a future release.

@@ -977,6 +978,7 @@ Settings used by the Enterprise Console (UI)
### GraphqlOptions


Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
Deprecated. The GraphQL feature will be removed in a future release.
Deprecated: The GraphQL feature will be removed in a future release.

@@ -599,7 +599,7 @@ type Route_DelegateAction struct {
}

type Route_GraphqlApiRef struct {
// Enterprise-Only: THIS FEATURE IS IN TECH PREVIEW. APIs are versioned as alpha and subject to change.
// Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE APIs are versioned as alpha and subject to change.

Choose a reason for hiding this comment

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

Suggested change
// Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE APIs are versioned as alpha and subject to change.
// Deprecated, Enterprise-Only: THIS FEATURE WILL BE REMOVED IN A FUTURE RELEASE. APIs are versioned as alpha and subject to change.

@@ -268,7 +268,10 @@ type Settings struct {
UpstreamOptions *UpstreamOptions `protobuf:"bytes,32,opt,name=upstreamOptions,proto3" json:"upstreamOptions,omitempty"`
// Enterprise-only: Settings for the Gloo Edge Enterprise Console (UI)
ConsoleOptions *ConsoleOptions `protobuf:"bytes,35,opt,name=console_options,json=consoleOptions,proto3" json:"console_options,omitempty"`
// Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
// Deprecated. The GraphQL feature will be removed in a future release.
// Deprecated: The GraphQL feature will be removed in a future release.

@@ -1312,6 +1316,7 @@ func (x *GatewayOptions) GetTranslateEmptyGateways() *wrapperspb.BoolValue {
return nil
}

// Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
// Deprecated. The GraphQL feature will be removed in a future release.
// Deprecated: The GraphQL feature will be removed in a future release.

@@ -1373,6 +1378,7 @@ func (x *ConsoleOptions) GetApiExplorerEnabled() *wrapperspb.BoolValue {
return nil
}

// Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
// Deprecated. The GraphQL feature will be removed in a future release.
// Deprecated: The GraphQL feature will be removed in a future release.

@@ -2802,7 +2808,10 @@ type Settings_DiscoveryOptions_FdsOptions struct {
sizeCache protoimpl.SizeCache
unknownFields protoimpl.UnknownFields

// Deprecated. The GraphQL feature will be removed in a future release.

Choose a reason for hiding this comment

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

Suggested change
// Deprecated. The GraphQL feature will be removed in a future release.
// Deprecated: The GraphQL feature will be removed in a future release.

@sheidkamp sheidkamp changed the title Sheidkamp/deprecate graphql Deprecate GraphQL Nov 15, 2024
Copy link

@sam-heilbron sam-heilbron left a comment

Choose a reason for hiding this comment

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

LGTM! Should we also make a note on https://docs.solo.io/gloo-edge/latest/guides/graphql/?

@sheidkamp
Copy link
Author

LGTM! Should we also make a note on https://docs.solo.io/gloo-edge/latest/guides/graphql/?

Added a warning at the top and a note to https://github.com/solo-io/solo-projects/issues/7161 to include the guides.

@sheidkamp
Copy link
Author

/kick (unclear failure)

@sheidkamp sheidkamp merged commit b9287ef into main Nov 15, 2024
17 of 18 checks passed
@sheidkamp sheidkamp deleted the sheidkamp/deprecate-graphql branch November 15, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants