Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed function getAppArmorFS #115749

Merged
merged 6 commits into from Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 1 addition & 47 deletions pkg/security/apparmor/validate.go
Expand Up @@ -17,11 +17,8 @@ limitations under the License.
package apparmor

import (
"bufio"
"errors"
"fmt"
"os"
"path"
"strings"

"github.com/opencontainers/runc/libcontainer/apparmor"
Expand All @@ -30,7 +27,6 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/features"
utilpath "k8s.io/utils/path"
)

// Whether AppArmor should be disabled by default.
Expand All @@ -48,20 +44,11 @@ func NewValidator() Validator {
if err := validateHost(); err != nil {
return &validator{validateHostErr: err}
}
appArmorFS, err := getAppArmorFS()
if err != nil {
return &validator{
validateHostErr: fmt.Errorf("error finding AppArmor FS: %v", err),
}
}
return &validator{
appArmorFS: appArmorFS,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like with this change Validator.ValidateHost interface becomes too trivial to keep. I'd suggest to remove it as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bart0sh, since some tests were panicking, after discussing with SergeyKanzhelev, I made a few changes and the interface is being used.

return &validator{}
}

type validator struct {
validateHostErr error
appArmorFS string
}

func (v *validator) Validate(pod *v1.Pod) error {
Expand Down Expand Up @@ -117,36 +104,3 @@ func validateHost() error {

return nil
}

func getAppArmorFS() (string, error) {
mountsFile, err := os.Open("/proc/mounts")
if err != nil {
return "", fmt.Errorf("could not open /proc/mounts: %v", err)
}
defer mountsFile.Close()

scanner := bufio.NewScanner(mountsFile)
for scanner.Scan() {
fields := strings.Fields(scanner.Text())
if len(fields) < 3 {
// Unknown line format; skip it.
continue
}
if fields[2] == "securityfs" {
appArmorFS := path.Join(fields[1], "apparmor")
if ok, err := utilpath.Exists(utilpath.CheckFollowSymlink, appArmorFS); !ok {
msg := fmt.Sprintf("path %s does not exist", appArmorFS)
if err != nil {
return "", fmt.Errorf("%s: %v", msg, err)
}
return "", errors.New(msg)
}
return appArmorFS, nil
}
}
if err := scanner.Err(); err != nil {
return "", fmt.Errorf("error scanning mounts: %v", err)
}

return "", errors.New("securityfs not found")
}
23 changes: 1 addition & 22 deletions pkg/security/apparmor/validate_test.go
Expand Up @@ -27,25 +27,6 @@ import (
"github.com/stretchr/testify/assert"
)

func TestGetAppArmorFS(t *testing.T) {
// This test only passes on systems running AppArmor with the default configuration.
// The test should be manually run if modifying the getAppArmorFS function.
t.Skip()

const expectedPath = "/sys/kernel/security/apparmor"
actualPath, err := getAppArmorFS()
assert.NoError(t, err)
assert.Equal(t, expectedPath, actualPath)
}

func TestValidateHost(t *testing.T) {
// This test only passes on systems running AppArmor with the default configuration.
// The test should be manually run if modifying the getAppArmorFS function.
t.Skip()

assert.NoError(t, validateHost())
}

func TestValidateBadHost(t *testing.T) {
hostErr := errors.New("expected host error")
v := &validator{
Expand All @@ -72,9 +53,7 @@ func TestValidateBadHost(t *testing.T) {
}

func TestValidateValidHost(t *testing.T) {
freddie400 marked this conversation as resolved.
Show resolved Hide resolved
v := &validator{
appArmorFS: "./testdata/",
}
v := &validator{}

tests := []struct {
profile string
Expand Down