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

Give better error if namespaces not enabled #399

Merged
merged 1 commit into from Nov 20, 2020
Merged

Give better error if namespaces not enabled #399

merged 1 commit into from Nov 20, 2020

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Nov 19, 2020

If the user is running Consul OSS by accident it would be nice to give them a better error message than:

2020-11-19T16:12:17.514Z [INFO]  Bootstrap token is provided so skipping Consul server ACL bootstrapping
2020-11-19T16:12:18.416Z [INFO]  Success: calling /agent/self to get datacenter
2020-11-19T16:12:18.416Z [INFO]  Current datacenter: datacenter=gcp-us-central-1
2020-11-19T16:12:18.610Z [INFO]  Policy "cross-namespace-policy" already exists, updating
2020-11-19T16:12:18.710Z [INFO]  Success: creating cross-namespace-policy policy
2020-11-19T16:12:18.711Z [ERROR] Error updating the default namespace to include the cross namespace policy: err="Unexpected response code: 404 ()"

How I've tested this PR: I didn't test it, I looked at the logs from an internal issue.

How I expect reviewers to test this PR: Code only. But possible to try and stand up a cluster with ACLs enabled, consul namespaces enabled and OSS binary.

Checklist:
No tests or changelog added because we don't test log messages and this isn't big enough for a changelog entry I don't think?

@lkysow lkysow requested review from a team, kschoche and ishustava and removed request for a team November 19, 2020 17:18
Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

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

looks great!

@lkysow lkysow merged commit d04cc0c into master Nov 20, 2020
@lkysow lkysow deleted the acl-ns-err branch November 20, 2020 00:54
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

3 participants