Draft: Exempt some Gitaly feature flags from needing a definition file
What does this MR do?
Feature flags in GitLab are now managed via definition files in [ee]/config/feature_flags/*.yml. In development and test mode - but not production - there are runtime checks on feature flags: if a flag is enabled or checked, then the definition file must be present, and certain fields (default_enabled and type) must match between the call and the definition. There's a long-term desire to remove those parameters from the calls, and source them solely from the definition. Additionally, in test mode, all feature flags are enabled.
This situation is an output of the feature flag working group, which I know @ayufan and @m_gill have been heavily involved in.
Gitaly uses the same feature flags, and FF architecture, as GitLab, prefixing its flags with gitaly_, but don't routinely create feature definition files for their flags. Whether the flags are enabled or not is communicated (IIRC) to Gitaly via gRPC metadata with every RPC call. So we get a few differences with non-Gitaly FFs:
- A feature flag may be enabled without ever being checked in the GitLab code
- Enabling a Gitaly feature flag in development mode (say, for manual testing) immediately starts raising exceptions
- In test mode, Gitaly feature flags behave as though they are disabled by default
Gitaly could be required to add feature flag definition files for each of their features, whether referenced / checked in the GitLab codebase or not. It would make it simple to enable all Gitaly FFs in tests, and solve the exception-raising situation, but it represents an overhead of two GitLab MRs per Gitaly feature flag, which seems onerous.
This MR takes a different approach, specifying that Gitaly feature flags do not need a definition file to be valid, as long as they are development feature flags that are not default_enabled: true. This represents the common case for feature flags. For ops FFs, or FFs that are enabled by default, we require a .yml file, although we have no way of enforcing it, since we don't know which FFs might or might not exist until someone creates one - and for Gitaly, that typically happens in production, rather than development. It does nothing for enabling gitalty FFs by default in tests, however.
Screenshots
This came up because I'm trying to add functionality to a Gitaly RPC (FetchRemote) which is currently being ported to Go. The Go implementation is hidden behind the gitaly_go_fetch_remote feature flag, which lacks a feature definition file. So I ran Feature.enable in GDK, and immediately got this error.
Right now, all I can do to get around the error is to create a feature definition file locally and never check it in, which seems like a silly state of affairs.