Update exec.go if needs a token

Like0x commented 1 year ago

Desired Outcome

Implemented Changes

This is configured using the GlobalBearerToken global variable

Connected Issue/Story

null

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be merged.

Changelog

[ ] The CHANGELOG has been updated, or
[ ] This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

[ ] This PR includes new unit and integration tests to go with the code changes, or
[ ] The changes in this PR do not require tests

Documentation

[ ] Docs (e.g. READMEs) were updated in this PR
[ ] A follow-up issue to update official docs has been filed here: [insert issue ID]
[ ] This PR does not require updating any documentation

Behavior

[ ] This PR changes product behavior and has been reviewed by a PO, or
[ ] These changes are part of a larger initiative that will be reviewed later, or
[ ] No behavior was changed with this PR

Security

[ ] Security architect has reviewed the changes in this PR,
[ ] These changes are part of a larger initiative with a separate security review, or
[ ] There are no security aspects to these changes

g3rzi commented 1 year ago

I am thinking maybe moving it to the InitHttpClient, because there are already similar check https://github.com/cyberark/kubeletctl/blob/4b4b989e59ea6b0a9e64c87cabf407a45c39b137/pkg/api/requests.go#L53-L55

g3rzi commented 1 year ago

Thank you for your contribution :)

Like0x commented 1 year ago

I am thinking maybe moving it to the InitHttpClient, because there are already similar check

https://github.com/cyberark/kubeletctl/blob/4b4b989e59ea6b0a9e64c87cabf407a45c39b137/pkg/api/requests.go#L53-L55

I found that --token-file parameter did not work properly on the authenticated 10250 port. So I observed the code and found that the config variable was re-assigned in exec.go. It did not inherit the previous configuration. So it did not carry the token. Since I am not very familiar with the project, I found that GlobalBearerToken can be used as a temporary judgment basis. If there is a better way, you can make modifications.

https://github.com/Like0x/kubeletctl/blob/173e12de55d46d0708e0627d303d6434846c36db/pkg/api/exec.go#L34-L43

g3rzi commented 1 year ago

Thank you for your clarification. For now, I merged your solution, and it looks good. But maybe in the future, we will need to do some modifications so it will be before the function.

Thanks again, I appreciate your contribution

cyberark / kubeletctl