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

Allow roles to be unmarshalled from bootstrap config #7036

Closed

Conversation

stefansedich
Copy link
Contributor

@stefansedich stefansedich commented May 25, 2021

It as not possible to use --bootstrap with a bootstrap file that contained roles as I was receiving the following error:

ERROR: cannot dynamically unmarshal resources of kind "role"

This PR adds support for unmarshalling roles from a bootstrap config file which appears to work but I might not understand some edge cases as to why this was not implemented so happy to be schooled 😅

I have done an end-to-end test loading our bootstrap.yaml which contains roles and it appears to function as expected. I also added an initial test around the bootstrap as I could not find any existing tests to add to, happy to tweak this if desired.

@russjones
Copy link
Contributor

@fspmarshall Is there a reason we did not add support for roles to the bootstrap flag?

@fspmarshall
Copy link
Contributor

@russjones Roles in --bootstrap have always been supported for Teleport Enterprise. At the time --bootstrap was implemented, OSS didn't have RBAC.

We can and should add support for roles in to OSS now that #5419 has landed. This PR will need to updated to ensure that --bootstrap enforces the same feature flags as normal role creation/upsert though. Also, we'll need to remove the marshaler/unmarshaler registration that currently lives in teleport.e.

@stefansedich
Copy link
Contributor Author

@russjones Roles in --bootstrap have always been supported for Teleport Enterprise. At the time --bootstrap was implemented, OSS didn't have RBAC.

We can and should add support for roles in to OSS now that #5419 has landed. This PR will need to updated to ensure that --bootstrap enforces the same feature flags as normal role creation/upsert though. Also, we'll need to remove the marshaler/unmarshaler registration that currently lives in teleport.e.

@fspmarshall happy to make the changes, any pointers as to where the feature flag changes will live? And what was teleport.e?

@fspmarshall
Copy link
Contributor

@stefansedich Oops, just realized that you were an outside contributor. Thanks for the PR!

teleport.e is the enterprise-specific component of the codebase that lives in a private submodule, which needs to be updated by a member of our team. This is a good start. Best bet is probably for one of us to cherry-pick these changes into another PR that ensures that the enterprise submodule gets appropriate updates in lock-step with these changes.

@stefansedich
Copy link
Contributor Author

stefansedich commented May 26, 2021

@stefansedich Oops, just realized that you were an outside contributor. Thanks for the PR!

teleport.e is the enterprise-specific component of the codebase that lives in a private submodule, which needs to be updated by a member of our team. This is a good start. Best bet is probably for one of us to cherry-pick these changes into another PR that ensures that the enterprise submodule gets appropriate updates in lock-step with these changes.

Ok gotcha thanks @fspmarshall ! in that case let me know if I can assist any further to land this one.

@stefansedich
Copy link
Contributor Author

stefansedich commented May 28, 2021

@fspmarshall is this something that might be able to land in 7.0? I am currently running our own custom build of teleport so we can advance our POC but would be great to not have to keep running that for too long.

@fspmarshall
Copy link
Contributor

@stefansedich 7.0 should be no problem.

Closing this PR in favor of #7130

@fspmarshall fspmarshall closed this Jun 1, 2021
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.

3 participants