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

Only manage allowed tenant flags #797

Merged
merged 4 commits into from
Jun 6, 2023
Merged

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Jun 6, 2023

🔧 Changes

There has been a recent uptick in incompatible tenant flags resulting in errors that resemble the following (source):

Payload validation error: 'Additional properties not allowed: new_universal_login_experience_enabled' on property flags 
(Flags used to change the behavior of this tenant).

The above example specifically calls out the new_universal_login_experience_enabled flag but this issue could occur for a handful of other properties too.

The issue here is that deprecations occur over time and sometimes these deprecations have a lag of rollout between environments and regions. What this means is that it is very possible for customers to have old config that successfully applies for one tenant but does not apply for another. The vast majority of the time, customers do not need to concern themselves with these tenant flags, they primarily exist to facilitate work behind the scenes.

This PR establishes an allow list of tenant flags that can be configured. This list is based off which flags are publicly exposed in the API docs. This PR simplifies the logic here, removing the concept of "migration" flags and simply checks against the list of allowed values. If an unallowed feature flag is present, it is removed from the payload. Additionally, logging has been added to educate and aid in understanding any edge-cases that may be missed. We very well need to tune this list as time goes on.

IMO, this is a medium-term fix. This list should remain largely static, should be backwards compatible and fix the immediate problems at hand. However, there is a broader problem of which flags can and should be considered configurable in the context of Deploy CLI. And if some of those are not publicly visible, how can we smooth over any breaking changes that can occur in the future? These are issues that cannot be addressed now but will discussed internally.

📚 References

🔬 Testing

Adding unit tests and re-recording E2E tests.

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

Sorry, something went wrong.

willvedd added 4 commits June 6, 2023 11:36

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
mcalhoun Matt Calhoun
@willvedd willvedd requested a review from a team as a code owner June 6, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants