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

fix: powershell does not support $? operator, or # for comments #70

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

troyfactor4
Copy link

Issue #, if available:

No issue, but I can create one if necessary.

Description of changes:

cfn-signal fails to execute as powershell does not recognise "$?" and instead on Windows we need to use "$LASTEXITCODE". The error given in the logs is: "cfn-signal: error: option -e: invalid integer value: '$?'"

Also starting a command with "#" will also fail. This seems like it was intended to be a comment in the logs, so used "echo" instead.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@troyfactor4 troyfactor4 changed the title fix: powershell does not support 0 operator, or # for comments fix: powershell does not support $? operator, or # for comments Feb 20, 2023
@dbaileyut
Copy link

I think the $? vs $LASTEXITCODE change is necessary.
Updating the # comment to echo is fine but, I don't see anything wrong with # - that's a valid comment in PowerShell.

@troyfactor4
Copy link
Author

The problem with the "#" is the way its being used is it is trying to execute a program called "#". If you look at the log files, something akin to "The application '#' cannot be found" is displayed. Now, it doesn't stop execution of the script but you get a bunch of unnecessary warning messages. Using "echo" achieves the intended purpose.

@dbaileyut
Copy link

dbaileyut commented Oct 27, 2023

Actually, is it even running PowerShell... it's <script>...</script> not <powershell>...</powershell>

@lorengordon
Copy link

Actually, is it even running PowerShell... it's <script>...</script> not <powershell>...</powershell>

I think the changeset is correct, but the explanation is off. cmd shell does not support $? or # comments. But powershell does. The change is compatible with cmd shell.

@dbaileyut
Copy link

$LASTEXITCODE won't work in cmd shell - you want %ERRORLEVEL% for cmd

@troyfactor4
Copy link
Author

I don't have a background in Windows servers, so I don't know if its cmd or powershell, but I can say that I (painstakingly) took the time to deploy the LZA multiple times to arrive at this pull request and the values I suggest provide working and clean output. %ERRORLEVEL% may also work, I have no idea, but I can also guarantee you from actually deploying it that $LASTEXITCODE does indeed work.

@dbaileyut
Copy link

I've got a pretty strong Windows background. $LASTEXITCODE won't error but doesn't do what's intended if cfn-init.exe fails. %ERRORLEVEL% is definitely what was intended. I'd suggest opening a GitHub issue and or AWS support case since this PR hasn't gotten picked up. I would, but I'm not using DirectoryServices and just came across this looking for duplicates on a different issue,

@troyfactor4
Copy link
Author

Got it. Thanks for the clarification @dbaileyut ! That makes sense.

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.

3 participants