gitbls / sdm

Raspberry Pi SD Card Image Manager
MIT License
469 stars 48 forks source link

Difference between `loadparams` and `plugin_getargs` in custom plugin? #227

Closed thk686 closed 4 months ago

thk686 commented 4 months ago

I think I used a template from sdm and it includes loadparms. The docs discuss plugin_getargs. Are they both required or are they redundant?

gitbls commented 4 months ago

TLDR: Yes, they are both required.

Long version ICYC: loadparams is defined at the top of the template as a bash function. It does one thing:

    source $SDMPT/etc/sdm/sdm-readparams

sdm-readparams reads the file /etc/sdm/cparams and defines all the sdm configuration variables that were set according to the command line. It also sources /usr/local/sdm/sdm-cparse that has function definitions in it that are used in plugins, such as logtoboth and plugin_getargs.

Thus, without calling loadparams the functions logtoboth and plugin_getargs (plus many others) will not be defined and the plugin will fail.

thk686 commented 4 months ago

I ended up with plugin_getargs "$pfx" "$2" as I could not get it to work with the next two optional fields. But it works, so I moved on.

gitbls commented 4 months ago

I ended up with plugin_getargs "$pfx" "$2" as I could not get it to work with the next two optional fields. But it works, so I moved on.

A bit more insight...Note that you only need to do a plugin_getargs if the plugin actually takes any arguments. If it doesn't, just don't use it.

That said, if you want to make it easier to add the first argument to your plugin at sometime in the future, you should use something like this:

args="$2"
vldargs="|notanarg1|notanarg2|"
rqdargs="|notanarg2|"
plugin_getargs $pfx "$args" "$vldargs" "$rqdargs" || exit

Then, you can replace notanargN by the real arguments as you need them. rqdargs setting is a way to ensure that if an argument is required, it's presence gets checked by plugin_getargs so you don't have to worry about it. If none of the arguments are required, then "" works fine (see, for example, /usr/local/sdm/plugins/piapps).

But, in the end, if what you implemented works, that's great.

thk686 commented 4 months ago

I guessed that is how it works. I probably mangled the formatting when calling the plugin.

On Sun, Jun 30, 2024 at 9:29 AM Benn @.***> wrote:

I ended up with plugin_getargs "$pfx" "$2" as I could not get it to work with the next two optional fields. But it works, so I moved on.

A bit more insight...Note that you only need to do a plugin_getargs if the plugin actually takes any arguments. If it doesn't, just don't use it.

That said, if you want to make it easier to add the first argument to your plugin at sometime in the future, you should use something like this:

args="$2" vldargs="|notanarg1|notanarg2|" rqdargs="|notanarg2|" plugin_getargs $pfx "$args" "$vldargs" "$rqdargs" || exit

Then, you can replace notanargN by the real arguments as you need them. rqdargs setting is a way to ensure that if an argument is required, it's presence gets checked by plugin_getargs so you don't have to worry about it. If none of the arguments are required, then "" works fine (see, for example, /usr/local/sdm/plugins/piapps).

But, in the end, if what you implemented works, that's great.

— Reply to this email directly, view it on GitHub https://github.com/gitbls/sdm/issues/227#issuecomment-2198581121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQXSLC4ZST45RAFHQ33SLZKAI2ZAVCNFSM6AAAAABKDSKELWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJYGU4DCMJSGE . You are receiving this because you modified the open/close state.Message ID: @.***>

-- Timothy H. Keitt www keittlab org

thk686 commented 4 months ago

Can plugin_getargs mangle characters? I need to pass a wireguard public key and the final "=" is missing.

thk686 commented 4 months ago

I see output like:

* Plugins selected:
   * disables
       Args: piwiz
   * user
       Args: deluser=pi
   * user
       Args: adduser=biosense|password=biosense
   * apps
       Args: apps=wireguard,audacity,alsa-utils,sox
   * bootconfig
       Args: dtoverlay=i2c-rtc,ds3231
   * sdm-biosense-setup-plugin
       Args: wg_ip=10.123.0.3|server_ip=192.168.1.21|server_public_key=815JJkrmg9DcD7biV0s+iQTFeCjvG8I84kwttKteM2s=

but logging inside my plugin gives:

* Plugin sdm-biosense-setup-plugin
wg_ip=10.123.0.3
server_ip=192.168.1.21
server_public_key=815JJkrmg9DcD7biV0s+iQTFeCjvG8I84kwttKteM2s

which shows the trailing = is removed.

The plugin code is:

#!/bin/bash

# Load SDM parameters
function loadparams() {
    source "$SDMPT/etc/sdm/sdm-readparams"
}

# Function to add a note to the end of the customization process
function plugin_addnote() {
    local note="$1"
    echo "$note" >> /etc/sdm/history
    echo "$note"
}

# Get the phase (0, 1, or post-install) and arguments
phase=$1
pfx="$(basename "$0")"
args="$2"
vldargs="|wg_ip|server_ip|server_public_key|"
reqargs="|wg_ip|server_ip|server_public_key|"
loadparams

# Extract arguments
plugin_getargs "$pfx" "$args" "$vldargs" "$reqargs" || exit

# Log the values for debugging
logtoboth "* Plugin $pfx"
logtoboth "wg_ip=${wg_ip}"
logtoboth "server_ip=${server_ip}"
logtoboth "server_public_key=${server_public_key}"
...
thk686 commented 4 months ago

I would suggest

function plugin_getargs() {
    #
    # Handles input data in the form: arg1=val1|arg2=val2|arg3=val3| ...
    # $1: Plugin name
    # $2: Argument list
    # $3: [optional] list of valid keys. Validity not checked if ""
    # $4: [optional] list of required keys. Required keys not checked if ""
    #
    # In addition to creating a key/value symbol for each found key/value,
    # also creates symbol foundkeys="|key1|key2|...|"
    #
    local arglist=() pfx=$1 largs="$2" validkeys="$3" rqkeys="$4" keysfound=""
    IFS="|" read -r -a arglist <<< "$largs"
    for c in "${arglist[@]}"
    do
        # Put the args into variables
        IFS="=" read -r key value remain <<< "$c"
        if [ "$remain" != "" ]; then
            value="${value}=${remain}"
        fi
        if [ "$validkeys" != "" ]
        then
            if ! [[ "$validkeys" =~ "|$key|" ]]
            then
                logtoboth "? Plugin $pfx: Unrecognized key '$key' in argument list '$largs'"
                return 1
            fi
        fi
        if [ "${key#\#}" != "$key" -o "${key#\\n}" != "$key" ]
        then
            # Handle a comment string that starts with '#' or '\n'. Prefix \n-led string with \n#
            [ "${key#\\n}" != "$key" ] && key="\n#${key#\\n}" 
            printf -v "comment" "%s" "$key"
            keysfound="${keysfound}|comment"   # Mark as 'comment' found
        else
            # Turn word-word into word__word. User of the value must handle other end for display
            [ "$key" != "${key/-/_}" ] && key="${key//-/__}"
            [ "$key" != "" ] && printf -v "${key}" "%s" "$value"
            keysfound="${keysfound}|$key"
        fi
    done
    #
    # Check required keys
    #
    if [ "$rqkeys" != "" ]
    then
        # Strip leading "|" from rqkeys to avoid checking for a null first arg
        IFS="|:" read -a arglist <<< "${rqkeys#|}"
        for c in "${arglist[@]}"
        do
            if ! [[ "${keysfound}|" =~ "|$c|" ]]
            then
                logtoboth "? Plugin $pfx: Required key '$c' missing from argument list '$largs'"
                return 1
            fi
        done
    fi
    # Strip leading "|" to avoid null string on subsequent splitting, but put one on the end
    [ "$keysfound" == "" ] && eval "foundkeys=\"\"" || eval "foundkeys=\"${keysfound#|}|\""
    return 0
}
gitbls commented 4 months ago

Perhaps you could flag the lines you've changed somehow so that I can sort it out?

Be aware that = is a significant character.

Will be looking at this later today. Thx

gitbls commented 4 months ago

Sorry, was busy earlier. I would not change getargs to fix this issue for reasons that should be fairly obvious.

Here are two possible solutions for you:

thk686 commented 4 months ago
--- original_plugin_getargs.sh  2024-07-08 12:00:00.000000000 +0000
+++ modified_plugin_getargs.sh  2024-07-08 12:00:00.000000000 +0000
@@ -1,36 +1,35 @@
 function plugin_getargs() {
     #
     # Handles input data in the form: arg1=val1|arg2=val2|arg3=val3| ...
-    # $1: Plugin name
-    # $2: Argument list
-    # $3: [optional] list of valid keys. Validity not checked if ""
-    # $4: [optional] list of required keys. Required keys not checked if ""
+    # $1: Plugin name
+    # $2: Argument list
+    # $3: [optional] list of valid keys. Validity not checked if ""
+    # $4: [optional] list of required keys. Required keys not checked if ""
     #
     # In addition to creating a key/value symbol for each found key/value,
     # also creates symbol foundkeys="|key1|key2|...|"
     #
     local arglist=() pfx=$1 largs="$2" validkeys="$3" rqkeys="$4" keysfound=""
     IFS="|" read -r -a arglist <<< "$largs"
     for c in "${arglist[@]}"
     do
-        # Put the args into variables
-        IFS="=" read -r key value remain <<< "$c"
-        [ "$remain" != "" ] && value="${value}=${remain}"
-        if [ "$validkeys" != "" ]
-        then
-            if ! [[ "$validkeys" =~ "|$key|" ]]
-            then
-                logtoboth "? Plugin $pfx: Unrecognized key '$key' in argument list '$largs'"
-                return 1
-            fi
-        fi
-        if [ "${key#\#}" != "$key" -o "${key#\\n}" != "$key" ]
-        then
-            # Handle a comment string that starts with '#' or '\n'. Prefix \n-led string with \n#
-            [ "${key#\\n}" != "$key" ] && key="\n#${key#\\n}" 
-            printf -v "comment" "%s" "$key"
-            keysfound="${keysfound}|comment"   # Mark as 'comment' found
-        else
-            # Turn word-word into word__word. User of the value must handle other end for display
-            [ "$key" != "${key/-/_}" ] && key="${key//-/__}"
-            # Don't slash-quote dollar signs. Breaks $ in passwords. What breaks if this is disabled?
-            # [[ "$value" =~ "$" ]] && value=${value//$/\\$}    # Replace $ with \$
-            [ "$key" != "" ] && printf -v "${key}" "%s" "$value" #eval "${key}=\"$value\""
-            keysfound="${keysfound}|$key"
-        fi
+        # Put the args into variables
+        IFS="=" read -r key value remain <<< "$c"
+        if [ "$remain" != "" ]; then
+            value="${value}=${remain}"
+        fi
+        if [ "$validkeys" != "" ]
+        then
+            if ! [[ "$validkeys" =~ "|$key|" ]]
+            then
+                logtoboth "? Plugin $pfx: Unrecognized key '$key' in argument list '$largs'"
+                return 1
+            fi
+        fi
+        if [ "${key#\#}" != "$key" -o "${key#\\n}" != "$key" ]
+        then
+            # Handle a comment string that starts with '#' or '\n'. Prefix \n-led string with \n#
+            [ "${key#\\n}" != "$key" ] && key="\n#${key#\\n}" 
+            printf -v "comment" "%s" "$key"
+            keysfound="${keysfound}|comment"   # Mark as 'comment' found
+        else
+            # Turn word-word into word__word. User of the value must handle other end for display
+            [ "$key" != "${key/-/_}" ] && key="${key//-/__}"
+            [ "$key" != "" ] && printf -v "${key}" "%s" "$value"
+            keysfound="${keysfound}|$key"
+        fi
     done
     #
     # Check required keys
@@ -39,13 +38,11 @@
     then
         # Strip leading "|" from rqkeys to avoid checking for a null first arg
         IFS="|:" read -a arglist <<< "${rqkeys#|}"
         for c in "${arglist[@]}"
         do
             if ! [[ "${keysfound}|" =~ "|$c|" ]]
             then
                 logtoboth "? Plugin $pfx: Required key '$c' missing from argument list '$largs'"
                 return 1
             fi
         done
     fi
     # Strip leading "|" to avoid null string on subsequent splitting, but put one on the end
     [ "$keysfound" == "" ] && eval "foundkeys=\"\"" || eval "foundkeys=\"${keysfound#|}|\""
     return 0
 }
gitbls commented 4 months ago

Thanks, but I just went through all the changed lines to see EXACTLY what you changed, and I'm not seeing it. What exactly do you think you did? Oh, I see, you removed some code that is required. Definitely not taking this approach.

thk686 commented 4 months ago

I found a workaround, so I am closing this.