Skip to content

Commit 44b704f

Browse files
steve-hbsteve-hb
committed
Fix misbehaviour in resources and file permissions; Stabilize tests and improve coverage
1 parent d7d64dc commit 44b704f

File tree

11 files changed

+200
-71
lines changed

11 files changed

+200
-71
lines changed

‎internal/provider/data/test/file_data_source_test.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,6 @@ func TestAccFileDataSource(t *testing.T) {
6969

7070
func testAccFileDataSourceConfig(path string) string {
7171
return fmt.Sprintf(`
72-
terraform {
73-
required_providers {
74-
ssh = {
75-
source = "askrella/ssh"
76-
version = "0.1.0"
77-
}
78-
}
79-
}
80-
81-
provider "askrella-ssh" {}
82-
8372
data "ssh_file_info" "test" {
8473
ssh = {
8574
host = "localhost"

‎internal/provider/resource/directory_resource.go

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (r *DirectoryResource) Create(ctx context.Context, req resource.CreateReque
149149
}
150150
defer client.Close()
151151

152-
permissions := parsePermissions(plan.Permissions.ValueString())
152+
permissions := ssh.ParsePermissions(plan.Permissions.ValueString())
153153

154154
if exists, _ := client.Exists(ctx, plan.Path.ValueString()); !exists {
155155
err = client.CreateDirectory(ctx, plan.Path.ValueString(), os.FileMode(permissions))
@@ -334,23 +334,11 @@ func (r *DirectoryResource) Update(ctx context.Context, req resource.UpdateReque
334334
}
335335
defer client.Close()
336336

337-
exists, err := client.Exists(ctx, plan.Path.ValueString())
338-
if err != nil {
339-
resp.Diagnostics.AddError(
340-
"Error determining if directory exists",
341-
fmt.Sprintf("Could determine directory existence: %s", err),
342-
)
343-
return
344-
}
345-
if !exists {
346-
resp.State.RemoveResource(ctx)
347-
return
348-
}
349-
350-
permissions := parsePermissions(plan.Permissions.ValueString())
337+
permissions := ssh.ParsePermissions(plan.Permissions.ValueString())
338+
wantedFileMode := os.FileMode(permissions)
351339

352340
if exists, _ := client.Exists(ctx, plan.Path.ValueString()); !exists {
353-
err = client.CreateDirectory(ctx, plan.Path.ValueString(), os.FileMode(permissions))
341+
err = client.CreateDirectory(ctx, plan.Path.ValueString(), wantedFileMode)
354342
if err != nil {
355343
resp.Diagnostics.AddError(
356344
"Error updating directory",
@@ -360,6 +348,24 @@ func (r *DirectoryResource) Update(ctx context.Context, req resource.UpdateReque
360348
}
361349
}
362350

351+
fileMode, err := client.GetFileMode(ctx, plan.Path.ValueString())
352+
if err != nil {
353+
resp.Diagnostics.AddError(
354+
"Error retrieving permissions",
355+
fmt.Sprintf("Could not retrieve permissions: %s", err),
356+
)
357+
}
358+
if fileMode != wantedFileMode {
359+
err := client.SetFileMode(ctx, plan.Path.ValueString(), wantedFileMode)
360+
if err != nil {
361+
resp.Diagnostics.AddError(
362+
"Error updating permissions",
363+
fmt.Sprintf("Could not set permissions: %s", err),
364+
)
365+
return
366+
}
367+
}
368+
363369
// Set ownership if specified
364370
if !plan.Owner.IsNull() || !plan.Group.IsNull() {
365371
err = client.SetFileOwnership(ctx, plan.Path.ValueString(), &ssh.FileOwnership{

‎internal/provider/resource/file_resource.go

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -163,19 +163,40 @@ func (r *FileResource) Create(ctx context.Context, req resource.CreateRequest, r
163163
return
164164
}
165165
if exists {
166-
resp.State.RemoveResource(ctx)
167-
return
166+
content, err := client.ReadFile(ctx, plan.Path.ValueString())
167+
if err != nil {
168+
resp.Diagnostics.AddError(
169+
"Error checking file content",
170+
fmt.Sprintf("Could not read file content: %s", err),
171+
)
172+
return
173+
}
174+
175+
// When content does not match the desired state, delete the file and pretend it doesn't exist (anymore)
176+
if content != plan.Content.ValueString() {
177+
err := client.DeleteFile(ctx, plan.Path.ValueString())
178+
if err != nil {
179+
resp.Diagnostics.AddError(
180+
"Error recreating file",
181+
fmt.Sprintf("Could delete file after content mismatch: %s", err),
182+
)
183+
return
184+
}
185+
exists = false
186+
}
168187
}
169188

170-
permissions := parsePermissions(plan.Permissions.ValueString())
189+
permissions := ssh.ParsePermissions(plan.Permissions.ValueString())
171190

172-
err = client.CreateFile(ctx, plan.Path.ValueString(), plan.Content.ValueString(), os.FileMode(permissions))
173-
if err != nil {
174-
resp.Diagnostics.AddError(
175-
"Error creating file",
176-
fmt.Sprintf("Could not create file: %s", err),
177-
)
178-
return
191+
if !exists {
192+
err = client.CreateFile(ctx, plan.Path.ValueString(), plan.Content.ValueString(), os.FileMode(permissions))
193+
if err != nil {
194+
resp.Diagnostics.AddError(
195+
"Error creating file",
196+
fmt.Sprintf("Could not create file: %s", err),
197+
)
198+
return
199+
}
179200
}
180201

181202
// Set ownership if specified
@@ -368,12 +389,16 @@ func (r *FileResource) Update(ctx context.Context, req resource.UpdateRequest, r
368389
)
369390
return
370391
}
371-
if !exists {
372-
resp.State.RemoveResource(ctx)
373-
return
392+
if exists {
393+
if err := client.DeleteFile(ctx, plan.Path.ValueString()); err != nil {
394+
resp.Diagnostics.AddError(
395+
"Error updating file",
396+
fmt.Sprintf("Could not recreate file: %s", err),
397+
)
398+
}
374399
}
375400

376-
permissions := parsePermissions(plan.Permissions.ValueString())
401+
permissions := ssh.ParsePermissions(plan.Permissions.ValueString())
377402

378403
err = client.CreateFile(ctx, plan.Path.ValueString(), plan.Content.ValueString(), os.FileMode(permissions))
379404
if err != nil {

‎internal/provider/resource/test/directory_resource_test.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@ import (
1717
func TestAccDirectoryResource(t *testing.T) {
1818
t.Parallel()
1919

20-
// Setup SSH client for verification
21-
sshConfig := ssh.SSHConfig{
22-
Host: "localhost",
23-
Port: 2222,
24-
Username: "testuser",
25-
Password: "testpass",
26-
}
27-
2820
client, err := ssh.NewSSHClient(context.Background(), sshConfig)
2921
require.NoError(t, err)
3022
defer client.Close()
@@ -62,7 +54,7 @@ func TestAccDirectoryResource(t *testing.T) {
6254
return fmt.Errorf("failed to get directory permissions: %v", err)
6355
}
6456
if mode != os.FileMode(0755) {
65-
return fmt.Errorf("unexpected permissions for creation: got %o, want 0755", mode)
57+
return fmt.Errorf("unexpected permissions for creation: got octal %o, want 0755", mode)
6658
}
6759

6860
// Verify ownership
@@ -83,10 +75,10 @@ func TestAccDirectoryResource(t *testing.T) {
8375
},
8476
// Update testing
8577
{
86-
Config: testAccDirectoryResourceConfig(dirName, "0775", "testuser", "testuser"),
78+
Config: testAccDirectoryResourceConfig(dirName, "0600", "testuser", "testuser"),
8779
Check: resource.ComposeAggregateTestCheckFunc(
8880
resource.TestCheckResourceAttr("ssh_directory.test", "path", testDirPath),
89-
resource.TestCheckResourceAttr("ssh_directory.test", "permissions", "0775"),
81+
resource.TestCheckResourceAttr("ssh_directory.test", "permissions", "0600"),
9082
resource.TestCheckResourceAttr("ssh_directory.test", "owner", "testuser"),
9183
resource.TestCheckResourceAttr("ssh_directory.test", "group", "testuser"),
9284
resource.TestCheckResourceAttr("ssh_directory.test", "ssh.host", "localhost"),
@@ -107,8 +99,8 @@ func TestAccDirectoryResource(t *testing.T) {
10799
if err != nil {
108100
return fmt.Errorf("failed to get directory permissions: %v", err)
109101
}
110-
if mode != os.FileMode(0775) {
111-
return fmt.Errorf("unexpected permissions for updating: got %o, want 0775", mode)
102+
if mode != os.FileMode(0600) {
103+
return fmt.Errorf("unexpected permissions for updating: got %o, want 0600", mode)
112104
}
113105

114106
// Verify ownership

‎internal/provider/resource/test/file_resource_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,6 @@ import (
1515
func TestAccFileResource(t *testing.T) {
1616
t.Parallel()
1717

18-
// Setup SSH client for verification
19-
sshConfig := ssh.SSHConfig{
20-
Host: "localhost",
21-
Port: 2222,
22-
Username: "testuser",
23-
Password: "testpass",
24-
}
25-
2618
client, err := ssh.NewSSHClient(context.Background(), sshConfig)
2719
require.NoError(t, err)
2820
defer client.Close()

‎internal/provider/resource/test/util_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package test
22

33
import (
44
"github.com/askrella/askrella-ssh-provider/internal/provider"
5+
"github.com/askrella/askrella-ssh-provider/internal/provider/ssh"
56
"github.com/hashicorp/terraform-plugin-framework/providerserver"
67
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
78
)
@@ -10,4 +11,11 @@ var (
1011
testAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){
1112
"ssh": providerserver.NewProtocol6WithError(provider.New("test")()),
1213
}
14+
15+
sshConfig = ssh.SSHConfig{
16+
Host: "::1",
17+
Port: 2222,
18+
Username: "testuser",
19+
Password: "testpass",
20+
}
1321
)

‎internal/provider/ssh/host_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package ssh
2+
3+
import (
4+
. "github.com/onsi/gomega"
5+
"net"
6+
"net/url"
7+
"testing"
8+
)
9+
10+
func TestHostParsing(t *testing.T) {
11+
RegisterTestingT(t)
12+
addresses := []string{
13+
"127.0.0.1", "localhost", "2a02:4f8:d014:b2f2::1",
14+
}
15+
16+
for _, addr := range addresses {
17+
t.Run(addr, func(t *testing.T) {
18+
RegisterTestingT(t)
19+
20+
ip := net.ParseIP(addr)
21+
if ip != nil {
22+
return
23+
}
24+
25+
_, err := url.Parse(addr)
26+
Expect(err).ToNot(HaveOccurred())
27+
})
28+
}
29+
}

‎internal/provider/ssh/ssh_client.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,20 @@ func (c *SSHClient) GetFileMode(ctx context.Context, path string) (os.FileMode,
258258
return info.Mode().Perm(), nil
259259
}
260260

261+
// GetFileMode gets the permissions of a file or directory
262+
func (c *SSHClient) SetFileMode(ctx context.Context, path string, mode os.FileMode) error {
263+
ctx, span := otel.Tracer("ssh-provider").Start(ctx, "SetFileMode")
264+
defer span.End()
265+
266+
err := c.SftpClient.Chmod(path, mode)
267+
if err != nil {
268+
c.logger.WithContext(ctx).WithError(err).Error("Failed to set file mode")
269+
return fmt.Errorf("failed to set file mode: %w", err)
270+
}
271+
272+
return nil
273+
}
274+
261275
// GetFileOwnership gets the user and group ownership of a file or directory
262276
func (c *SSHClient) GetFileOwnership(ctx context.Context, path string) (*FileOwnership, error) {
263277
ctx, span := otel.Tracer("ssh-provider").Start(ctx, "GetFileOwnership")

‎internal/provider/ssh/ssh_client_test.go

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,68 @@ package ssh
33
import (
44
"context"
55
"crypto/rand"
6-
. "github.com/onsi/gomega"
76
"os"
87
"path"
98
"testing"
9+
10+
. "github.com/onsi/gomega"
1011
)
1112

13+
var sshConfig = SSHConfig{
14+
Host: "localhost",
15+
Port: 2222,
16+
Username: "testuser",
17+
Password: "testpass",
18+
}
19+
20+
func TestFilePermissions(t *testing.T) {
21+
RegisterTestingT(t)
22+
23+
client, err := NewSSHClient(context.Background(), sshConfig)
24+
Expect(err).ToNot(HaveOccurred())
25+
ctx := context.Background()
26+
basePath := "/home/testuser/ssh_test_" + rand.Text()
27+
28+
testCases := []struct {
29+
name string
30+
filePath string
31+
content string
32+
permissions os.FileMode
33+
}{
34+
{
35+
name: "Test File Permissions 0777",
36+
filePath: basePath + "_1",
37+
content: "Hello World",
38+
permissions: 0777,
39+
},
40+
{
41+
name: "Test File Permissions 0644",
42+
filePath: basePath + "_2",
43+
content: "Hello World",
44+
permissions: 0644,
45+
},
46+
{
47+
name: "Test File Permissions 0600",
48+
filePath: basePath + "_3",
49+
content: "Hello World",
50+
permissions: 0600,
51+
},
52+
}
53+
54+
for _, tc := range testCases {
55+
t.Run(tc.name, func(t *testing.T) {
56+
RegisterTestingT(t)
57+
58+
Expect(client.CreateFile(ctx, tc.filePath, tc.content, tc.permissions)).Should(Succeed())
59+
Expect(client.GetFileMode(ctx, tc.filePath)).To(BeEquivalentTo(tc.permissions))
60+
})
61+
}
62+
}
63+
1264
func TestDirectoryOperations(t *testing.T) {
1365
RegisterTestingT(t)
1466

15-
client, err := NewSSHClient(context.Background(), SSHConfig{
16-
Host: "localhost",
17-
Port: 2222,
18-
Username: "testuser",
19-
Password: "testpass",
20-
})
67+
client, err := NewSSHClient(context.Background(), sshConfig)
2168
Expect(err).ToNot(HaveOccurred())
2269

2370
basePath := "/home/testuser/ssh_test_" + rand.Text()

‎internal/provider/resource/util.go renamed to ‎internal/provider/ssh/util.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
package resource
1+
package ssh
22

33
import "strconv"
44

5-
func parsePermissions(perms string) uint32 {
5+
func ParsePermissions(perms string) uint32 {
66
if perms == "" {
77
return 0644
88
}

‎internal/provider/ssh/util_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package ssh
2+
3+
import (
4+
. "github.com/onsi/gomega"
5+
"testing"
6+
)
7+
8+
func TestParsePermissions(t *testing.T) {
9+
RegisterTestingT(t)
10+
11+
tests := []struct {
12+
str string
13+
expected uint32
14+
}{
15+
{"755", 0755},
16+
{"0755", 0755},
17+
{"777", 0777},
18+
{"0777", 0777},
19+
{"0600", 0600},
20+
{"600", 0600},
21+
}
22+
for _, test := range tests {
23+
t.Run(test.str, func(t *testing.T) {
24+
Expect(ParsePermissions(test.str)).To(Equal(test.expected))
25+
})
26+
}
27+
}

0 commit comments

Comments
 (0)