Ignore all annotations for a given volume if any required field is missing.

Mount annotations require that `share`, `type` and `source` fields are all
specified via annotations. If any one is missing, then all the remaining
annotations for a given volume should be ignored. Earlier we would error out.
Now we just log a warning.

PiperOrigin-RevId: 524332461
This commit is contained in:
Ayush Ranjan
2023-04-14 10:56:50 -07:00
committed by gVisor bot
parent 927e99ebaa
commit 96bc3756b0
2 changed files with 50 additions and 59 deletions
+4 -8
View File
@@ -107,14 +107,10 @@ func newPodMountHints(spec *specs.Spec) (*podMountHints, error) {
// Validate all the parsed hints.
for name, m := range mnts {
log.Infof("Mount annotation found, name: %s, source: %q, type: %s, share: %v", name, m.mount.Source, m.mount.Type, m.share)
if m.share == invalid {
return nil, fmt.Errorf("share field for %q has not been set", m.name)
}
if len(m.mount.Source) == 0 {
return nil, fmt.Errorf("source field for %q has not been set", m.name)
}
if len(m.mount.Type) == 0 {
return nil, fmt.Errorf("type field for %q has not been set", m.name)
if m.share == invalid || len(m.mount.Source) == 0 || len(m.mount.Type) == 0 {
log.Warningf("ignoring mount annotations for %q because of missing required field(s)", name)
delete(mnts, name)
continue
}
// Check for duplicate mount sources.
+46 -51
View File
@@ -96,57 +96,6 @@ func TestPodMountHintsErrors(t *testing.T) {
},
error: "invalid mount name",
},
{
name: "missing source",
annotations: map[string]string{
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "pod",
},
error: "source field",
},
{
name: "missing type",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.share": "pod",
},
error: "type field",
},
{
name: "missing share",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.type": "tmpfs",
},
error: "share field",
},
{
name: "invalid source",
annotations: map[string]string{
MountPrefix + "mount1.source": "",
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "pod",
},
error: "source field for \"mount1\" has not been set",
},
{
name: "invalid type",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.type": "invalid-type",
MountPrefix + "mount1.share": "pod",
},
error: "type field for \"mount1\" has not been set",
},
{
name: "invalid share",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "invalid-share",
},
error: "share field for \"mount1\" has not been set",
},
{
name: "duplicate source",
annotations: map[string]string{
@@ -174,6 +123,52 @@ func TestPodMountHintsErrors(t *testing.T) {
}
}
// Tests that when a required mount annotation is missing, the entire mount
// hint is omitted and ignored.
func TestPodMountHintsIgnore(t *testing.T) {
for _, tst := range []struct {
name string
annotations map[string]string
}{
{
name: "invalid source",
annotations: map[string]string{
MountPrefix + "mount1.source": "",
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "pod",
},
},
{
name: "invalid type",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.type": "invalid",
MountPrefix + "mount1.share": "pod",
},
},
{
name: "invalid share",
annotations: map[string]string{
MountPrefix + "mount1.source": "foo",
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "invalid",
},
},
} {
t.Run(tst.name, func(t *testing.T) {
spec := &specs.Spec{Annotations: tst.annotations}
podHints, err := newPodMountHints(spec)
if err != nil {
t.Errorf("newPodMountHints() failed: %v", err)
} else if podHints != nil {
if hint, ok := podHints.mounts["mount1"]; ok {
t.Errorf("hint was provided when it should have been omitted: %+v", hint)
}
}
})
}
}
func TestHintsCheckCompatible(t *testing.T) {
for _, tc := range []struct {
name string