Prevent non-core-feature elements from being marked @inaccessible if referenced by core feature elements

[sachindshinde]

So currently, the following supergraph schema does not error when being converted to an API schema:

schema
  @link(url: "https://specs.apollo.dev/link/v1.0")
  @link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION)
  @link(url: "https://specs.apollo.dev/inaccessible/v0.2", for: SECURITY)
  @link(url: "http://localhost/foo/v1.0")
{
  query: Query
}

# Omitting core feature element definitions for core, join, and inaccessible for brevity.

input InputObject {
  someField: String
  privateField: String @inaccessible
}

# Has a default value that references the @inaccessible input field InputObject.privateField
directive @foo(someArg: InputObject = { privateField: "" }) on FIELD

type Query @join__type(graph: SUBGRAPH) {
  foo: String
}

This is because removeInaccessibleElements() is aware that core feature elements aren't exposed in the API schema, and won't error when an @inaccessible non-core feature element is referenced by a core feature element.

Currently, no Apollo core spec to my knowledge introduces core feature elements that can reference non-built-in, non-core feature elements. And since composition doesn't allow user-defined core specs at the moment, that means nothing should be relying on this behavior for now.

However:

1. I'm guessing in the near future we're going to allow user-defined core specs into the supergraph schema during composition, and we currently don't seem to have validation forbidding core feature elements from referencing non-built-in, non-core feature elements.
2. We're introducing a mechanism for exposing core feature directives in the API schema in PR for surfacing @tag and other core directives to the supergraph #1760

So in order to prevent exposed core feature directives from also exposing @inaccessible feature elements in the API schema, we need to either:

1. Update removeInaccessibleElements() to be aware of which core feature elements are exposed in the API schema.
2. Update removeInaccessibleElements() to forbid non-core-feature elements from being marked @inaccessible if referenced by core feature elements.

I've arbitrarily chosen Option 2 in this PR because:

1. It was easier to code/I'm crunched for time for Contracts GA 😛
2. I'm not sure how people feel about this core-referencing-non-core thing to start with, and with no one currently relying on it, this would be the time to forbid it.

I'd also be fine with Option 1 (e.g. if folks want to leave the possibility open for this kind of referencing in user-defined core specs). Regardless of what we choose, the main thing I'd like to prevent is the leaking of @inaccessible feature elements in the API schema.

Note that the error messaging for when this edge case occurs isn't great in this PR (it tells the user the referencing element is in the API schema, regardless of whether it actually is). I could return a trilean enum from isInAPISchema() (using the third value for "maybe") and update messaging if folks would prefer, or some other means of improving the messaging.

[netlify]

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	fcc4bf0

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[pcmanus]

I'm not sure how people feel about this core-referencing-non-core thing to start with

Personally, I'm strongly in favour of not allowing such a thing, probably forever, but at a minimum initially. If we find a convincing use case for such a thing some day, we can discuss then.

Anyway, as core feature are currently essentially hard-coded, we essentially have no real validation on them, but we will eventually need to have some and validate that. In the meantime, totally agree with the simple and pragmatic fix of this PR.