这是indexloc提供的服务，不要输入任何密码
Skip to content

option to disable metadata and graphql apis (close #1088) #1650

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Feb 28, 2019
Merged

option to disable metadata and graphql apis (close #1088) #1650

merged 12 commits into from
Feb 28, 2019

Conversation

@nizar-m
Copy link
Contributor

@nizar-m nizar-m commented Feb 22, 2019 •
edited
Loading

Description

Currently both the Metadata (/api/1/table and /v1/query) and the GraphQL endpoints (/v1alpha1/graphql) are enabled by default.

A new flag --enabled-apis (EnvVar: HASURA_GRAPHQL_ENABLED_APIS) is introduced.

  • Accepts comma list of APIs to be enabled
    • --enabled-apis metadata enables Metadata
    • --enabled-apis graphql enabled GraphQL
    • --enabled-apis metadata,graphql enables both
  • By default, both Metadata and GraphQL APIs will be enabled

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#1088

Solution and Design

  • Parse EnvVar HASURA_GRAPHQL_ENABLED_APIS as a comma separated list of APIs to be enabled
  • Parse flag --enabled-apis
  • Disable /v1/query, /v1/template, /api/1/table, and console if RQL is disabled
  • Disable /v1alpha1/graphql if GraphQL is disabled

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@nizar-m nizar-m requested a review from rikinsk-zz as a code owner February 22, 2019 09:44
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 22, 2019

Review app for commit 1e334ff deployed to Heroku: https://hge-ci-pull-1650.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1650-1e334ff

0x777
0x777 reviewed Feb 25, 2019
View reviewed changes
server/src-lib/Hasura/Server/App.hs Outdated

-- get "v1alpha1/graphql/schema" $
-- mkSpockAction encodeQErr serverCtx v1Alpha1GQSchemaHandler
when enableRQL $ do
Copy link
Member

@0x777 0x777 Feb 25, 2019

Choose a reason for hiding this comment

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

Let's put this under the earlier when enableRQL block

@0x777
Copy link
Member

0x777 commented Feb 25, 2019

@nizar-m After a bit of discussion, we want to call these apis instead of interfaces and the RQL interface as metadata. So, the env variable would be HASURA_GRAPHQL_ALLOWED_APIS. By default it is, HASURA_GRAPHQL_ALLOWED_APIS=metadata,graphql. Also regarding the command line flag, should we just call it --allowed-apis which will accept comma separated values? What is the typical practice? @shahidhk @rikinsk

@shahidhk
Copy link
Member

shahidhk commented Feb 25, 2019

HASURA_GRAPHQL_ALLOWED_APIS=metadata,graphql as env var and --allowed-apis=metadata,graphl as flag is recommended.

@shahidhk shahidhk added c/server Related to server c/docs Related to docs labels Feb 25, 2019
@shahidhk
Copy link
Member

shahidhk commented Feb 25, 2019

I'd also suggest changing ALLOWED toENABLED in-lieu with ENABLE_CONSOLE. What rings to me when using ALLOWED is ALLOWED_ROLES

So, HASURA_GRAPHQL_ENABLED_APIS=metadata,graphql as env var and --enabled-apis=metadata,graphl as flag.

shahidhk
shahidhk reviewed Feb 26, 2019
View reviewed changes
server/src-lib/Hasura/Server/Init.hs Outdated
readAPIs :: String -> Either String [API]
readAPIs = mapM readAPI . T.splitOn "," . T.pack
where readAPI si = case T.toUpper $ T.strip si of
"RQL" -> Right RQL
Copy link
Member

@shahidhk shahidhk Feb 26, 2019

Choose a reason for hiding this comment

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

We need to change RQL to METADATA

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 26, 2019

Review app for commit beed128 deployed to Heroku: https://hge-ci-pull-1650.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1650-beed128

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 26, 2019

Review app for commit fb27d8e deployed to Heroku: https://hge-ci-pull-1650.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1650-fb27d8e

@shahidhk shahidhk changed the title Option to disable RQL/GraphQL (close #1088) option to disable metadata and graphql apis (close #1088) Feb 27, 2019
@shahidhk shahidhk requested a review from 0x777 February 28, 2019 05:56
0x777
0x777 previously approved these changes Feb 28, 2019
View reviewed changes
rikinsk-zz
rikinsk-zz suggested changes Feb 28, 2019
View reviewed changes
shahidhk
shahidhk approved these changes Feb 28, 2019
View reviewed changes
Copy link
Member

@shahidhk shahidhk left a comment

Choose a reason for hiding this comment

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

LGTM

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 28, 2019

Review app for commit acb56cc deployed to Heroku: https://hge-ci-pull-1650.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1650-acb56cc

@shahidhk shahidhk requested a review from rikinsk-zz February 28, 2019 13:50
@shahidhk shahidhk merged commit 1fa66dc into hasura:master Feb 28, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 28, 2019

Review app https://hge-ci-pull-1650.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Feb 19, 2025
@danieljharvey @hasura-bot
Scalar arrays in OpenDD filters (#1650)
1f95d11 
<!-- The PR description should answer 2 important questions: -->

### What

Make these work, functional no-op.

V3_GIT_ORIGIN_REV_ID: 92b0fb21da4fc3a9a540c6be58038e4e593ecca4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahidhk shahidhk shahidhk approved these changes

@0x777 0x777 0x777 approved these changes

+1 more reviewer

@rikinsk-zz rikinsk-zz rikinsk-zz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c/docs Related to docs c/server Related to server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nizar-m @hasura-bot @0x777 @shahidhk @rikinsk-zz