Skip to content

Commit 9a93f26

Browse files
committed
Fix copilot review comments from blockdev mounts PR
Addresses the following comments: microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) Assisted-by: GitHub Copilot:claude-opus-4.7 copilot-review Signed-off-by: Tingmao Wang <m@maowtm.org>
1 parent 643766d commit 9a93f26

5 files changed

Lines changed: 16 additions & 7 deletions

File tree

internal/guest/runtime/hcsv2/uvm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ func checkContainerSettings(sandboxID, containerID string, settings *prot.VMHost
408408
func (h *Host) checkMountsNotBroken() error {
409409
if h.HasSecurityPolicy() && h.mountsBroken.Load() {
410410
return errors.Errorf(
411-
"Mount, unmount, container creation and deletion has been disabled in this UVM due to a previous error (%q)",
411+
"Mount, unmount, container creation and deletion have been disabled in this UVM due to a previous error (%q)",
412412
h.mountsBrokenCausedBy,
413413
)
414414
}

internal/regopolicyinterpreter/regopolicyinterpreter_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,11 @@ func Test_copyRegoMetadata(t *testing.T) {
8989
if copyObject, ok := copy[name]; ok {
9090
if !assertObjectsEqual(origObject, copyObject) {
9191
t.Errorf("original and copy differ on key %s", name)
92+
return false
9293
}
9394
} else {
9495
t.Errorf("copy missing object %s", name)
96+
return false
9597
}
9698
}
9799

pkg/securitypolicy/regopolicy_linux_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,7 +2127,7 @@ func Test_Rego_EnforceCreateContainer_Capabilities_Drop_NoMatches(t *testing.T)
21272127
}
21282128
}
21292129

2130-
func Test_Regi_EnforceCreateContainer_RequireNoDevices(t *testing.T) {
2130+
func Test_Rego_EnforceCreateContainer_RequireNoDevices(t *testing.T) {
21312131
f := func(p *generatedConstraints) bool {
21322132
tc, err := setupSimpleRegoCreateContainerTest(p)
21332133
if err != nil {
@@ -2156,7 +2156,7 @@ func Test_Regi_EnforceCreateContainer_RequireNoDevices(t *testing.T) {
21562156
}
21572157

21582158
if err := quick.Check(f, &quick.Config{MaxCount: 50, Rand: testRand}); err != nil {
2159-
t.Errorf("Test_Regi_EnforceCreateContainer_RequireNoDevices: %v", err)
2159+
t.Errorf("Test_Rego_EnforceCreateContainer_RequireNoDevices: %v", err)
21602160
}
21612161
}
21622162

pkg/securitypolicy/securitypolicyenforcer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func init() {
6060

6161
// Represents an in-progress revertable section. To ensure state is consistent,
6262
// Commit() and Rollback() must not fail, so they do not return anything, and if
63-
// an error does occur they should panic.
63+
// an error does occur they should panic or trigger an unrecoverable error.
6464
type RevertableSectionHandle interface {
6565
Commit()
6666
Rollback()

pkg/securitypolicy/securitypolicyenforcer_rego.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,13 @@ func appendMountData(mountData []interface{}, mounts []oci.Mount) []interface{}
843843
return mountData
844844
}
845845

846+
func uint32ptrtoany(i *uint32) interface{} {
847+
if i == nil {
848+
return nil
849+
}
850+
return *i
851+
}
852+
846853
func appendDeviceData(deviceData []interface{}, devices []oci.LinuxDevice) []interface{} {
847854
for _, device := range devices {
848855
deviceData = append(deviceData, inputData{
@@ -851,8 +858,8 @@ func appendDeviceData(deviceData []interface{}, devices []oci.LinuxDevice) []int
851858
"major": device.Major,
852859
"minor": device.Minor,
853860
"fileMode": device.FileMode,
854-
"uid": device.UID,
855-
"gid": device.GID,
861+
"uid": uint32ptrtoany(device.UID),
862+
"gid": uint32ptrtoany(device.GID),
856863
})
857864
}
858865

@@ -1245,7 +1252,7 @@ func (policy *regoEnforcer) StartRevertableSection() (RevertableSectionHandle, e
12451252
if err != nil {
12461253
err = errors.Wrapf(err, "unable to save metadata for revertable section")
12471254
policy.revertableSectionLock.Unlock()
1248-
return &revertableSectionHandle{}, err
1255+
return nil, err
12491256
}
12501257
// Keep policy.revertableSectionLock locked until the end of the section.
12511258
sh := &revertableSectionHandle{

0 commit comments

Comments
 (0)