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

Add spoc merge Command #2136

Merged
merged 3 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
31 changes: 31 additions & 0 deletions cmd/spoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"sigs.k8s.io/security-profiles-operator/cmd"
spocli "sigs.k8s.io/security-profiles-operator/internal/pkg/cli"
"sigs.k8s.io/security-profiles-operator/internal/pkg/cli/merger"
"sigs.k8s.io/security-profiles-operator/internal/pkg/cli/puller"
"sigs.k8s.io/security-profiles-operator/internal/pkg/cli/pusher"
"sigs.k8s.io/security-profiles-operator/internal/pkg/cli/recorder"
Expand Down Expand Up @@ -78,6 +79,22 @@ func main() {
},
},
},
&cli.Command{
Name: "merge",
Aliases: []string{"m"},
Usage: "merge multiple security profiles",
Action: merge,
ArgsUsage: "INFILE...",
Flags: []cli.Flag{
&cli.StringFlag{
Name: merger.FlagOutputFile,
Aliases: []string{"o"},
Usage: "the output file path for the combined profile",
DefaultText: merger.DefaultOutputFile,
TakesFile: true,
},
},
},
&cli.Command{
Name: "run",
Aliases: []string{"x"},
Expand Down Expand Up @@ -192,6 +209,20 @@ func record(ctx *cli.Context) error {
return nil
}

// merge runs the `spoc merge` subcommand.
func merge(ctx *cli.Context) error {
options, err := merger.FromContext(ctx)
if err != nil {
return fmt.Errorf("build options: %w", err)
}

if err := merger.New(options).Run(); err != nil {
return fmt.Errorf("launch merger: %w", err)
}

return nil
}

// run runs the `spoc run` subcommand.
func run(ctx *cli.Context) error {
options, err := runner.FromContext(ctx)
Expand Down
27 changes: 27 additions & 0 deletions internal/pkg/cli/merger/consts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
Copyright 2023 The Kubernetes Authors.
mhils marked this conversation as resolved.
Show resolved Hide resolved

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.
*/

package merger

import "sigs.k8s.io/security-profiles-operator/internal/pkg/cli"

// DefaultOutputFile defines the default output location for the merger.
var DefaultOutputFile = cli.DefaultFile

const (
// FlagOutputFile is the flag for defining the output file location.
FlagOutputFile string = cli.FlagOutputFile
)
38 changes: 38 additions & 0 deletions internal/pkg/cli/merger/impl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright 2023 The Kubernetes 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.
*/

package merger

import (
"os"
)

type defaultImpl struct{}

//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -header ../../../../hack/boilerplate/boilerplate.generatego.txt
//counterfeiter:generate . impl
type impl interface {
ReadFile(string) ([]byte, error)
WriteFile(name string, data []byte, perm os.FileMode) error
}

func (*defaultImpl) ReadFile(name string) ([]byte, error) {
return os.ReadFile(name)
}

func (*defaultImpl) WriteFile(name string, data []byte, perm os.FileMode) error {
return os.WriteFile(name, data, perm)
}
106 changes: 106 additions & 0 deletions internal/pkg/cli/merger/merger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
Copyright 2023 The Kubernetes 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.
*/

package merger

import (
"bytes"
"fmt"
"log"

"k8s.io/cli-runtime/pkg/printers"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"

seccompprofileapi "sigs.k8s.io/security-profiles-operator/api/seccompprofile/v1beta1"
selinuxprofileapi "sigs.k8s.io/security-profiles-operator/api/selinuxprofile/v1alpha2"
"sigs.k8s.io/security-profiles-operator/internal/pkg/manager/recordingmerger"
)

// Merger is the main structure of this package.
type Merger struct {
impl
options *Options
}

// New returns a new Merger instance.
func New(options *Options) *Merger {
return &Merger{
impl: &defaultImpl{},
options: options,
}
}

// Run the Merger.
func (p *Merger) Run() error {
log.Printf("Merging %d profiles into %s", len(p.options.inputFiles), p.options.outputFile)

contents := make([]client.Object, len(p.options.inputFiles))
for i, filepath := range p.options.inputFiles {
log.Printf("Reading file %s", filepath)
content, err := p.ReadFile(filepath)
if err != nil {
return fmt.Errorf("open profile: %w", err)
}

// yaml.Unmarshal happily takes YAML for a SELinux profile and unmarshals
// it into SeccompProfile. We need to check the YAML kind!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This here was quite surprising to me and annoying to track down.

I think this is also a bug in Artifact.Pull(...), but I haven't verified it end-to-end. If we all are happy with directly using yalm.Unmarshal there (instead of going through a.YamlUnmarshal), I'm happy to follow up with another PR that extracts profile loading into a generic utility function which is used in both places.
In any case, that's definitely a separate PR. :)

Copy link
Member

Choose a reason for hiding this comment

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

We only used if for seccomp right now, so yeah. This may be a bad bug.

var genericCRD map[string]interface{}
err = yaml.Unmarshal(content, &genericCRD)
if err != nil {
fmt.Println(err)
return fmt.Errorf("cannot parse yaml: %w", err)
}
kind, ok := genericCRD["kind"].(string)
if !ok {
return fmt.Errorf("invalid yaml, kind missing: %w", err)
}

switch kind {
case "SeccompProfile":
var profile seccompprofileapi.SeccompProfile
if err := yaml.Unmarshal(content, &profile); err != nil {
return fmt.Errorf("unmarshal to seccomp profile: %w", err)
}
contents[i] = &profile
case "SelinuxProfile":
var profile selinuxprofileapi.SelinuxProfile
if err := yaml.Unmarshal(content, &profile); err != nil {
return fmt.Errorf("unmarshal to selinux profile: %w", err)
}
contents[i] = &profile
default:
return fmt.Errorf("unexpected YAML kind: %s", kind)
}
}

merged, err := recordingmerger.MergeProfiles(contents)
if err != nil {
return fmt.Errorf("merge profiles: %w", err)
}

var buffer bytes.Buffer
printer := printers.YAMLPrinter{}
if err := printer.PrintObj(merged, &buffer); err != nil {
return fmt.Errorf("print YAML: %w", err)
}
const filePermissions = 0o600
if err := p.WriteFile(p.options.outputFile, buffer.Bytes(), filePermissions); err != nil {
return fmt.Errorf("failed to write output file: %w", err)
}

return nil
}
159 changes: 159 additions & 0 deletions internal/pkg/cli/merger/merger_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
//go:build linux && !no_bpf
// +build linux,!no_bpf

/*
Copyright 2023 The Kubernetes 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.
*/

package merger

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/require"

"sigs.k8s.io/security-profiles-operator/internal/pkg/cli/merger/mergerfakes"
)

const SeccompA = `
apiVersion: security-profiles-operator.x-k8s.io/v1beta1
kind: SeccompProfile
spec:
defaultAction: SCMP_ACT_ERRNO
syscalls:
- action: SCMP_ACT_ALLOW
names:
- foo
`

const SeccompB = `
apiVersion: security-profiles-operator.x-k8s.io/v1beta1
kind: SeccompProfile
spec:
defaultAction: SCMP_ACT_ERRNO
syscalls:
- action: SCMP_ACT_ALLOW
names:
- bar
`

const SeccompMerged = `apiVersion: security-profiles-operator.x-k8s.io/v1beta1
kind: SeccompProfile
metadata:
creationTimestamp: null
spec:
defaultAction: SCMP_ACT_ERRNO
syscalls:
- action: SCMP_ACT_ALLOW
names:
- foo
- action: SCMP_ACT_ALLOW
names:
- bar
status: {}
`

const SelinuxA = `
apiVersion: security-profiles-operator.x-k8s.io/v1alpha2
kind: SelinuxProfile
spec:
inherit:
- name: container
allow:
var_log_t:
dir:
- open
`

func TestRun(t *testing.T) {
t.Parallel()

defaultOptions := func() *Options {
options := Default()
options.inputFiles = []string{"foo.yaml", "bar.yaml"}
return options
}

for _, tc := range []struct {
name string
prepare func(*mergerfakes.FakeImpl) *Options
assert func(*mergerfakes.FakeImpl, error)
}{
{
name: "successful seccomp merge",
prepare: func(mock *mergerfakes.FakeImpl) *Options {
mock.ReadFileReturnsOnCall(0, []byte(SeccompA), nil)
mock.ReadFileReturnsOnCall(1, []byte(SeccompB), nil)
return defaultOptions()
},
assert: func(mock *mergerfakes.FakeImpl, err error) {
require.Nil(t, err)
_, merged, _ := mock.WriteFileArgsForCall(0)
require.Equal(t, SeccompMerged, string(merged))
},
},
{
name: "cannot merge different formats",
prepare: func(mock *mergerfakes.FakeImpl) *Options {
mock.ReadFileReturnsOnCall(0, []byte(SeccompA), nil)
mock.ReadFileReturnsOnCall(1, []byte(SelinuxA), nil)
return defaultOptions()
},
assert: func(mock *mergerfakes.FakeImpl, err error) {
require.ErrorContains(t, err, "cannot merge SeccompProfile with *recordingmerger.MergeableSelinuxProfile")
require.Equal(t, 0, mock.WriteFileCallCount())
},
},
{
name: "input file not found",
prepare: func(mock *mergerfakes.FakeImpl) *Options {
mock.ReadFileReturnsOnCall(0, nil, errors.New("file not found"))
return defaultOptions()
},
assert: func(mock *mergerfakes.FakeImpl, err error) {
require.ErrorContains(t, err, "open profile: file not found")
},
},
{
name: "input file is not yaml",
prepare: func(mock *mergerfakes.FakeImpl) *Options {
mock.ReadFileReturnsOnCall(0, []byte("% this is not yaml"), nil)
return defaultOptions()
},
assert: func(mock *mergerfakes.FakeImpl, err error) {
require.ErrorContains(t, err, "cannot parse yaml")
},
},
} {
prepare := tc.prepare
assert := tc.assert

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

mock := &mergerfakes.FakeImpl{}
options := prepare(mock)

sut := New(options)
sut.impl = mock

err := sut.Run()
fmt.Println("err", err)
assert(mock, err)
})
}
}
Loading
Loading