kc9wwh / macOSUpgrade

Workflow for doing in-place upgrades.
Other
418 stars 103 forks source link

Improve validations and startosinstall switches #70

Closed taniguti closed 5 years ago

taniguti commented 5 years ago

Hi,

kc9wwh commented 5 years ago

@taniguti Everything looks good, but I'm hesitant with the changes to the checksum verification. A lot of users do not use this feature, so not allowing it to be blank worries me that folks will run into issues cause they aren't aware of this change. We all no people don't read the release notes...

Was there a specific issue or reason this was not working?

verifyChecksum() {
    if [[ "$installESDChecksum" != "" ]]; then
        osChecksum=$( /sbin/md5 -q "$OSInstaller/Contents/SharedSupport/InstallESD.dmg" )
        if [[ "$osChecksum" == "$installESDChecksum" ]]; then
            /bin/echo "Checksum: Valid"
            validChecksum=1
            return
        else
            /bin/echo "Checksum: Not Valid"
            /bin/echo "Beginning new dowload of installer"
            /bin/rm -rf "$OSInstaller"
            /bin/sleep 2
            downloadInstaller
        fi
    else
        ##Checksum not specified as script argument, assume true
        validChecksum=1
        return
    fi
}
taniguti commented 5 years ago

I agree people never read the release notes.

On my changes of this PR, parameter 7 still allows blank. Previous merge, following parameters 8 and 9 added. Due to implementation of Jamf, position of parameters are important. In case of user wants to skip checksum verification and wants to use eraseInstall, there is no way to do so.

So the spacial word 'none' is equal to blank for the parameter 7. My description is not correct. The parameter 7, installESDChecksum will required ONLY when user want to use eraseInstall and/or userDialog to 1.

I updated comment in code.

MScottBlake commented 5 years ago

It is possible to pass a blank parameter on the command line by using quotes.

/path/to/script.sh 1 2 '' 4

taniguti commented 5 years ago

Will Jamf script deal blank field with using quotes?

kc9wwh commented 5 years ago

Yes, the Jamf binary will skip over blank script parameters and runs as expected. Did a test run to confirm.

vssEAU-macOS10:~ admin$ sudo jamf policy -trigger jssParamTest
Checking for policies triggered by "jssParamTest" for user "admin"...
Executing Policy jssParamTesting
Running script jssParamTesting...
Script exit code: 0
Script result: Paramater 1: /
Parameter 2: vssEAU-macOS10.13-OktaDuoTesting
Parameter 3: admin
Parameter 4: foo
Parameter 5: bar
Parameter 6: hola
Parameter 7: 
Parameter 8: welcome
Parameter 9: goodbye

Parameter 7 should be blank as above. If the variables were not properly quoted I would of expected 8 and 9 to jump ahead one.

taniguti commented 5 years ago

Thanks. It's good. I reverted codes around installESDChecksum.