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

Check file access information in conformance tests #5102

Merged
merged 4 commits into from
Oct 29, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions test/conformance/runtime/filesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ func testFiles(t *testing.T, clients *test.Clients, paths map[string]types.FileI
return fmt.Errorf("%s has invalid permissions string %s: %w", path, riFile.Perm, err)
}
}

riFileAccess, ok := ri.Host.FileAccess[path]
if ok && riFileAccess.ReadErr != "" {
return fmt.Errorf("Want no read errors for file %s, got error: %s", path, riFileAccess.ReadErr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("Want no read errors for file %s, got error: %s", path, riFileAccess.ReadErr)
return fmt.Errorf("want no read errors for file %s, got error: %s", path, riFileAccess.ReadErr)

}
if ok && riFileAccess.WriteErr != "" {
return fmt.Errorf("Want no write errors for file %s, got error: %s", path, riFileAccess.WriteErr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("Want no write errors for file %s, got error: %s", path, riFileAccess.WriteErr)
return fmt.Errorf("want no write errors for file %s, got error: %s", path, riFileAccess.WriteErr)

}
}
return nil
}
Expand Down
106 changes: 106 additions & 0 deletions test/test_images/runtime/handlers/file_access_attempt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
Copyright 2019 The Knative Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the copied license is not the correct format.
Empty line has to be there after line number 5 and 6

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch, we've fixed this now


package handlers

import (
"io/ioutil"
"os"

"knative.dev/serving/test/types"
)

type permissionBits uint32
Copy link
Contributor

Choose a reason for hiding this comment

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

https://golang.org/pkg/os/#FileMode should work just 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.

Yeah both FileMode and permissionBits are both unint32. Reason we went with custom type in this test case was to add the ability to compare permission bits via hasPermission function. This makes the code more readable.


const (
otherRead permissionBits = 1 << (2 - iota)
otherWrite
)

func (p permissionBits) hasPermission(mode permissionBits) bool {
return p&mode == mode
}

func fileAccessAttempt(filePaths ...string) map[string]types.FileAccessInfo {
files := map[string]types.FileAccessInfo{}
for _, path := range filePaths {
accessInfo := types.FileAccessInfo{}

if err := checkReadable(path); err != nil {
accessInfo.ReadErr = err.Error()
}

if err := checkWritable(path); err != nil {
accessInfo.WriteErr = err.Error()
}

files[path] = accessInfo
}
return files
}

// checkReadable function checks whether path file or directory is readable
func checkReadable(path string) error {
file, err := os.Stat(path) // It should only return error only if file does not exist
if err != nil {
return err
}

// We aren't expected to be able to read, so just exit
perm := permissionBits(file.Mode().Perm())
if !perm.hasPermission(otherRead) {
return nil
}

if file.IsDir() {
_, err := ioutil.ReadDir(path)
return err
}

readFile, err := os.OpenFile(path, os.O_RDONLY, 0)
if err != nil {
return err
}
return readFile.Close()
}

// checkWritable function checks whether path file or directory is writable
func checkWritable(path string) error {
file, err := os.Stat(path) // It should only return error only if file does not exist
if err != nil {
return err
}

// We aren't expected to be able to write, so just exits
perm := permissionBits(file.Mode().Perm())
if !perm.hasPermission(otherWrite) {
return nil
}

if file.IsDir() {
writeFile, err := ioutil.TempFile(path, "random")
if writeFile != nil {
os.Remove(writeFile.Name())
}
return err
}

writeFile, err := os.OpenFile(path, os.O_APPEND, 0)
shashwathi marked this conversation as resolved.
Show resolved Hide resolved
if writeFile != nil {
defer writeFile.Close()
}
return err
}
36 changes: 30 additions & 6 deletions test/test_images/runtime/handlers/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import (
"knative.dev/serving/test/types"
)

var fileAccessExclusions = []string{
"/dev/tty",
"/dev/console",
}

func runtimeHandler(w http.ResponseWriter, r *http.Request) {
log.Println("Retrieving Runtime Information")
w.Header().Set("Content-Type", "application/json")
Expand All @@ -38,14 +43,33 @@ func runtimeHandler(w http.ResponseWriter, r *http.Request) {
k := &types.RuntimeInfo{
Request: requestInfo(r),
Host: &types.HostInfo{EnvVars: env(),
Files: fileInfo(filePaths...),
Cgroups: cgroups(cgroupPaths...),
Mounts: mounts(),
Stdin: stdin(),
User: userInfo(),
Args: args(),
Files: fileInfo(filePaths...),
FileAccess: fileAccessAttempt(excludeFilePaths(filePaths, fileAccessExclusions)...),
Cgroups: cgroups(cgroupPaths...),
Mounts: mounts(),
Stdin: stdin(),
User: userInfo(),
Args: args(),
},
}

writeJSON(w, k)
}

func excludeFilePaths(filePaths []string, excludedPaths []string) []string {
var paths []string
for _, path := range filePaths {
excluded := false
for _, excludedPath := range excludedPaths {
if path == excludedPath {
excluded = true
break
}
}

if !excluded {
paths = append(paths, path)
}
}
return paths
}
12 changes: 12 additions & 0 deletions test/types/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ type HostInfo struct {
Files map[string]FileInfo `json:"files"`
// EnvVars is a map of all environment variables set.
EnvVars map[string]string `json:"envs"`
// FileAccess is a map of file access information
FileAccess map[string]FileAccessInfo `json:"fileaccess"`
// Cgroups is a list of cgroup information.
Cgroups []*Cgroup `json:"cgroups"`
// Mounts is a list of mounted volume information, or error.
Expand Down Expand Up @@ -189,6 +191,16 @@ type FileInfo struct {
Error string `json:"error,omitempty"`
}

// FileAccessInfo contains the file access information
type FileAccessInfo struct {
// ReadErr is the String representation of an error received when attempting to read
// a file or directory
ReadErr string `json:"read_error,omitempty"`
// WriteErr is the String representation of an error received when attempting to write
// to a file or directory
WriteErr string `json:"write_error,omitempty"`
}

// Cgroup contains the Cgroup value for a given setting.
type Cgroup struct {
// Name is the full path name of the cgroup.
Expand Down