-
Notifications
You must be signed in to change notification settings - Fork 155
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
fix: avoid erroring if conflicting techniques are set from environment variables #374
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,16 +110,32 @@ export default class Config { | |
core.getInput("webhook") || process.env.SLACK_WEBHOOK_URL || null, | ||
webhookType: core.getInput("webhook-type"), | ||
}; | ||
this.validate(); | ||
this.mask(); | ||
this.validate(core); | ||
core.debug(`Gathered action inputs: ${JSON.stringify(this.inputs)}`); | ||
this.content = new Content().get(this); | ||
core.debug(`Parsed request content: ${JSON.stringify(this.content)}`); | ||
} | ||
|
||
/** | ||
* Hide secret values provided in the inputs from appearing. | ||
*/ | ||
mask() { | ||
if (this.inputs.token) { | ||
core.debug("Setting the provided token as a secret variable."); | ||
core.setSecret(this.inputs.token); | ||
} | ||
if (this.inputs.webhook) { | ||
core.debug("Setting the provided webhook as a secret variable."); | ||
core.setSecret(this.inputs.webhook); | ||
} | ||
} | ||
|
||
/** | ||
* Confirm the configurations are correct enough to continue. | ||
* @param {core} core - GitHub Actions core utilities. | ||
*/ | ||
validate() { | ||
validate(core) { | ||
switch (this.inputs.retries.trim().toUpperCase()) { | ||
case this.Retries.ZERO: | ||
case this.Retries.FIVE: | ||
|
@@ -133,29 +149,20 @@ export default class Config { | |
); | ||
} | ||
switch (true) { | ||
case !!this.inputs.token && !!this.inputs.webhook: | ||
core.debug( | ||
"Setting the provided token and webhook as secret variables.", | ||
); | ||
core.setSecret(this.inputs.token); | ||
core.setSecret(this.inputs.webhook); | ||
case !!core.getInput("token") && !!core.getInput("webhook"): | ||
throw new SlackError( | ||
core, | ||
"Invalid input! Either the token or webhook is required - not both.", | ||
); | ||
case !!this.inputs.token: | ||
core.debug("Setting the provided token as a secret variable."); | ||
core.setSecret(this.inputs.token); | ||
if (!this.inputs.method) { | ||
case !!this.inputs.method: | ||
if (!this.inputs.token) { | ||
Comment on lines
+157
to
+158
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Special care was taken in flipping these cases with wording since we can't assume a set Instead, I think we must check for a |
||
throw new SlackError( | ||
core, | ||
"Missing input! A method must be decided to use the token provided.", | ||
"Missing input! A token must be provided to use the method decided.", | ||
); | ||
} | ||
break; | ||
case !!this.inputs.webhook: | ||
core.debug("Setting the provided webhook as a secret variable."); | ||
core.setSecret(this.inputs.webhook); | ||
if (!this.inputs.webhookType) { | ||
throw new SlackError( | ||
core, | ||
|
@@ -175,7 +182,7 @@ export default class Config { | |
default: | ||
throw new SlackError( | ||
core, | ||
"Missing input! Either a token or webhook is required to take action.", | ||
"Missing input! Either a method or webhook is required to take action.", | ||
); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ export default async function send(core) { | |
*/ | ||
async function post(config) { | ||
switch (true) { | ||
case !!config.inputs.token: | ||
case !!config.inputs.method: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO an important change that might've been missed without testing a bit! Before changes, an environment variable might've set this and caused this This is covered and confirmed in the |
||
return await new Client().post(config); | ||
case !!config.inputs.webhook: | ||
return await new Webhook().post(config); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents adding both
token
andwebhook
as inputs usingwith
while still allowing both environment variables!