Remove token botUser hack #483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cleanup logging, remove mostly wrong is global config logging in slackSend, replace with values that are safe to go into the log. Code simplification for getting config or descriptor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In preparation for removing the direct token support, I noticed that botUser is loaded conditionally from the global config if token is null, I assume this was previously overriding anyones job or step config if they set bot user to be something different to global and used credentials.
I think a more complete fix might be to create a slack credential type and then have the bot user on the credential, but that's a more involved changed and requires migration, more testing and some thought
The current fix will prevent setting it to false at the job level, functionality that is probably not needed and is pretty similar to what was there before but not gated with the is token set hack
Also:
Cleaned up logging, removed mostly wrong global config logging in
slackSend, replaced with values that are safe to go into the log.
Code simplification for getting config or descriptor