From 27dbcad5cdd732a4777e886806c5eeb1a06129a4 Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Sat, 9 Mar 2024 23:20:44 -0500 Subject: [PATCH] chore: minor cleanups and doc updates (#1108) * Fix some obvious errors, remove dead code * Add instructions for manually adding public key to authorized_keys file * Add GitHub context dump step and update testacc workflow condition --------- Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- .github/workflows/test.yml | 21 +++++++++++++-------- .vscode/settings.json | 3 +++ docs/index.md | 2 ++ proxmox/ssh/client.go | 24 ++++++++++++------------ proxmoxtf/resource/file.go | 11 ----------- proxmoxtf/resource/vm/vm.go | 2 +- 6 files changed, 31 insertions(+), 32 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d3678a87..3d65e981 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,18 +26,18 @@ jobs: - '**/*.go' - name: Setup Go - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} uses: actions/setup-go@v5 with: go-version-file: "go.mod" cache-dependency-path: "**/*.sum" - name: Get dependencies - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} run: go mod download - name: Build - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} run: go vet . && go build -v . test: @@ -45,6 +45,11 @@ jobs: needs: build runs-on: ubuntu-latest steps: + - name: Dump GitHub context + env: + GITHUB_CONTEXT: ${{ toJson(github) }} + run: echo "$GITHUB_CONTEXT" + - name: Checkout uses: actions/checkout@v4 with: @@ -65,11 +70,11 @@ jobs: cache-dependency-path: "**/*.sum" - name: Get dependencies - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} run: go mod download - name: Unit tests - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} timeout-minutes: 10 run: go test -v -cover ./... @@ -77,7 +82,7 @@ jobs: run: make docs && git diff --exit-code testacc: - if: "!contains(github.head_ref, 'renovate/') && !contains(github.head_ref, 'release-please') && github.repository == 'bpg/terraform-provider-proxmox'" + if: ${{ !contains(github.head_ref, 'renovate/') && !contains(github.head_ref, 'release-please') && !contains(github.head_ref, 'dependabot') && github.repository == 'bpg/terraform-provider-proxmox' }} name: Dispatch Acceptance Tests needs: build runs-on: ubuntu-latest @@ -96,9 +101,9 @@ jobs: - '**/*.go' - name: Invoke acceptance tests workflow - if: steps.filter.outputs.go == 'true' + if: ${{ steps.filter.outputs.go == 'true' }} uses: benc-uk/workflow-dispatch@v1 with: workflow: testacc.yml ref: ${{ github.event.pull_request.head.ref }} - inputs: '{"ref": "${{ github.head_ref }}" }' \ No newline at end of file + inputs: '{"ref": "${{ github.head_ref }}" }' diff --git a/.vscode/settings.json b/.vscode/settings.json index 2657b458..23d94852 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,6 +5,7 @@ "ACPI", "archlinux", "armhf", + "bodyclose", "burstable", "capi", "CDROM", @@ -33,6 +34,7 @@ "iothreads", "ivshmem", "keyctl", + "knownhosts", "mbps", "mdev", "nameserver", @@ -51,6 +53,7 @@ "rootfs", "seabios", "signoff", + "skeema", "SMBIOSSKU", "SMBIOSUUID", "stretchr", diff --git a/docs/index.md b/docs/index.md index d6953a52..2be9cb37 100644 --- a/docs/index.md +++ b/docs/index.md @@ -214,6 +214,8 @@ You can configure the `sudo` privilege for the user via the command line on the ssh-copy-id terraform@ ``` + or manually add your public key to the `~/.ssh/authorized_keys` file of the `terraform` user on the target node. + - Test the SSH connection and password-less `sudo`: ```sh diff --git a/proxmox/ssh/client.go b/proxmox/ssh/client.go index 5929943f..52714500 100644 --- a/proxmox/ssh/client.go +++ b/proxmox/ssh/client.go @@ -387,33 +387,33 @@ func (c *client) openNodeShell(ctx context.Context, node ProxmoxNode) (*ssh.Clie return nil, fmt.Errorf("failed to read %s: %w", khPath, err) } - // Create a custom permissive hostkey callback which still errors on hosts + // Create a custom permissive host key callback which still errors on hosts // with changed keys, but allows unknown hosts and adds them to known_hosts cb := ssh.HostKeyCallback(func(hostname string, remote net.Addr, key ssh.PublicKey) error { - kherr := kh(hostname, remote, key) - if knownhosts.IsHostKeyChanged(kherr) { + khErr := kh(hostname, remote, key) + if knownhosts.IsHostKeyChanged(khErr) { return fmt.Errorf("REMOTE HOST IDENTIFICATION HAS CHANGED for host %s! This may indicate a MitM attack", hostname) } - if knownhosts.IsHostUnknown(kherr) { - f, ferr := os.OpenFile(khPath, os.O_APPEND|os.O_WRONLY, 0o600) - if ferr == nil { + if knownhosts.IsHostUnknown(khErr) { + f, fErr := os.OpenFile(khPath, os.O_APPEND|os.O_WRONLY, 0o600) + if fErr == nil { defer utils.CloseOrLogError(ctx)(f) - ferr = knownhosts.WriteKnownHost(f, hostname, remote, key) + fErr = knownhosts.WriteKnownHost(f, hostname, remote, key) } - if ferr == nil { + if fErr == nil { tflog.Info(ctx, fmt.Sprintf("Added host %s to known_hosts", hostname)) } else { tflog.Error(ctx, fmt.Sprintf("Failed to add host %s to known_hosts", hostname), map[string]interface{}{ - "error": kherr, + "error": khErr, }) } return nil } - return kherr + return khErr }) tflog.Info(ctx, fmt.Sprintf("agent is set to %t", c.agent)) @@ -568,10 +568,10 @@ func (c *client) socks5SSHClient(sshServerAddress string, sshConfig *ssh.ClientC return nil, fmt.Errorf("failed to dial %s via SOCKS5 proxy %s: %w", sshServerAddress, c.socks5Server, err) } - sshConn, chans, reqs, err := ssh.NewClientConn(conn, sshServerAddress, sshConfig) + sshConn, ch, reqs, err := ssh.NewClientConn(conn, sshServerAddress, sshConfig) if err != nil { return nil, fmt.Errorf("failed to create SSH client connection: %w", err) } - return ssh.NewClient(sshConn, chans, reqs), nil + return ssh.NewClient(sshConn, ch, reqs), nil } diff --git a/proxmoxtf/resource/file.go b/proxmoxtf/resource/file.go index 1ce25ccf..352baa57 100644 --- a/proxmoxtf/resource/file.go +++ b/proxmoxtf/resource/file.go @@ -17,7 +17,6 @@ import ( "net/url" "os" "path/filepath" - "regexp" "sort" "strings" "time" @@ -29,7 +28,6 @@ import ( "golang.org/x/exp/slices" "github.com/bpg/terraform-provider-proxmox/proxmox/api" - "github.com/bpg/terraform-provider-proxmox/proxmox/ssh" "github.com/bpg/terraform-provider-proxmox/proxmoxtf" "github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validators" "github.com/bpg/terraform-provider-proxmox/utils" @@ -578,15 +576,6 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag return diags } - if err != nil { - if matches, e := regexp.MatchString(`cannot move .* Permission denied`, err.Error()); e == nil && matches { - return diag.FromErr(ssh.NewErrUserHasNoPermission(capi.SSH().Username())) - } - - diags = append(diags, diag.Errorf("error moving file: %s", err.Error())...) - - return diags - } } volID, di := fileGetVolumeID(d) diff --git a/proxmoxtf/resource/vm/vm.go b/proxmoxtf/resource/vm/vm.go index 59afd4d9..e08b59be 100644 --- a/proxmoxtf/resource/vm/vm.go +++ b/proxmoxtf/resource/vm/vm.go @@ -5193,7 +5193,7 @@ func vmUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D 0, true, ) - if err != nil { + if er != nil { return diag.FromErr(er) }