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

Do not sync Database CA if leaf 9.0 #12040

Merged
merged 3 commits into from
May 4, 2022
Merged

Conversation

jakule
Copy link
Contributor

@jakule jakule commented Apr 18, 2022

When in an existing trusted cluster the root cluster is being updated to v10 this warning message shows in the logs, as leaf still being in v9 doesn't know about Database CA introduced in v10:

2022-04-18T15:09:07-04:00 WARN [PROXY:SER] Failed to rotate external ca error:[
ERROR REPORT:
Original Error: *trace.BadParameterError 'db' authority type is not supported
Stack Trace:

Caught:
	/home/jnyckowski/projects/teleport/lib/httplib/httplib.go:142 github.com/gravitational/teleport/lib/httplib.ConvertResponse
	/home/jnyckowski/projects/teleport/lib/auth/clt.go:273 github.com/gravitational/teleport/lib/auth.(*Client).PostJSON
	/home/jnyckowski/projects/teleport/lib/auth/clt.go:439 github.com/gravitational/teleport/lib/auth.(*Client).RotateExternalCertAuthority
	/home/jnyckowski/projects/teleport/lib/reversetunnel/remotesite.go:502 github.com/gravitational/teleport/lib/reversetunnel.(*remoteSite).watchCertAuthorities
	/home/jnyckowski/projects/teleport/lib/reversetunnel/remotesite.go:435 github.com/gravitational/teleport/lib/reversetunnel.(*remoteSite).updateCertAuthorities
	/home/jnyckowski/sdk/go1.18/src/runtime/asm_amd64.s:1571 runtime.goexit
User Message: 'db' authority type is not supported
] cluster:example.com reversetunnel/remotesite.go:503

This PR add a version check to skip the Database CA sync if the leaf is in the lower version then v10, so this warning is not being printed.

@jakule jakule marked this pull request as ready for review April 18, 2022 20:20
@github-actions github-actions bot requested review from LKozlowski and r0mant April 18, 2022 20:20
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Code lgtm but please add test coverage before merging.

@r0mant r0mant requested review from smallinsky and removed request for LKozlowski April 20, 2022 02:01
@@ -1107,7 +1112,7 @@ func newRemoteSite(srv *server, domainName string, sconn ssh.Conn) (*remoteSite,
return nil, err
}

go remoteSite.updateCertAuthorities(caRetry)
go remoteSite.updateCertAuthorities(caRetry, remoteClusterVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: instead of forwarding remoteClusterVersion from two layers it can be just saved as a member in remoteSite struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about it, the remoteSite struct is already quite huge and looks like the version is not needed anywhere. Let me look at this once again.

@espadolini
Copy link
Contributor

Does this mean that database access in a v9 leaf cluster is not going to work when the root is v10?

@jakule
Copy link
Contributor Author

jakule commented Apr 21, 2022

@espadolini It will work. v9 is using HostCA to sign certificates in Database Access where v10 uses Database CA introduced in v10. For that reason Database CA should not be sent to v9 cluster as this one uses Host CA in database access and is not aware of Database CA.

jakule added 3 commits May 4, 2022 13:52

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@jakule jakule force-pushed the jakule/skip-database-ca-trusted-v9 branch from 191e8f1 to e346880 Compare May 4, 2022 18:28
@jakule jakule enabled auto-merge (squash) May 4, 2022 20:00
@jakule jakule merged commit 402acb8 into master May 4, 2022
@jakule jakule deleted the jakule/skip-database-ca-trusted-v9 branch May 4, 2022 21:10
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

4 participants