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

feat(kuma-dp) rename tokenPath metadata field name #1427

Conversation

tharun208
Copy link
Contributor

renamed dataplaneTokenPath to dataplane.token.filename in envoy metadata

Fix #512

Signed-off-by: Tharun rajendrantharun@live.com

@tharun208 tharun208 requested a review from a team as a code owner January 15, 2021 14:37
@tharun208 tharun208 force-pushed the feat/rename_dp_token_path_metdata_name branch from 71f09ad to a836501 Compare January 15, 2021 15:53
Copy link
Contributor

@nickolaev nickolaev left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

@@ -16,7 +16,7 @@ var metadataLog = core.Log.WithName("xds-server").WithName("metadata-tracker")
const (
// Supported Envoy node metadata fields.

fieldDataplaneTokenPath = "dataplaneTokenPath"
fieldDataplaneTokenPath = "dataplane.token.filename"
Copy link
Contributor

Choose a reason for hiding this comment

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

This change has to be backward compatible.

When CP uses this field, it should also support the old field

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean this will break the deployed kuma-dp's that already are preconfigured with the old name?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

renamed dataplaneTokenPath to dataplane.token.filename in envoy metadata
added backward compatibility

Fix kumahq#512

Signed-off-by: Tharun <rajendrantharun@live.com>
@@ -67,6 +67,9 @@ func DataplaneMetadataFromXdsMetadata(xdsMetadata *_struct.Struct) *DataplaneMet
if xdsMetadata == nil {
return &metadata
}
if field := xdsMetadata.Fields["dataplaneTokenPath"]; field != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakubdyszkiewicz does this change look's okay for backward compatibility ?.

@jakubdyszkiewicz
Copy link
Contributor

Thanks for the contribution and sorry for super late response. We don't use this field anymore, it was used for backward compatibility. It can be removed now.

@tharun208 tharun208 deleted the feat/rename_dp_token_path_metdata_name branch July 5, 2021 12:49
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.

Rename dataplaneTokenPath field from Envoy node metadata into dataplane.token.filename
3 participants