Refactor code for security policy (microsoft#1279)

The current implementation exposes some internal structure,
which is unnecessary as well as some structs are duplicated
across security policy package and security policy tool.
This PR refactors code to de-duplicate exported structures
and hides internal implementation behind new factory methods.

Signed-off-by: Maksim An <[email protected]>

tools/securitypolicy/main.go

@@ -7,8 +7,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"regexp"
-	"strconv"
 
 	"github.com/BurntSushi/toml"
 	"github.com/Microsoft/hcsshim/ext4/tar2ext4"
@@ -36,21 +34,18 @@ func main() {
 			return err
 		}
 
-		config := &Config{
-			AllowAll:   false,
-			Containers: []Container{},
-		}
+		config := &securitypolicy.PolicyConfig{}
 
 		err = toml.Unmarshal(configData, config)
 		if err != nil {
 			return err
 		}
 
-		policy, err := func() (securitypolicy.SecurityPolicy, error) {
+		policy, err := func() (*securitypolicy.SecurityPolicy, error) {
 			if config.AllowAll {
-				return createOpenDoorPolicy(), nil
+				return securitypolicy.NewOpenDoorPolicy(), nil
 			} else {
-				return createPolicyFromConfig(*config)
+				return createPolicyFromConfig(config)
 			}
 		}()
 
@@ -77,190 +72,87 @@ func main() {
 	}
 }
 
-type EnvironmentVariableRule struct {
-	Strategy securitypolicy.EnvVarRule `toml:"strategy"`
-	Rule     string                    `toml:"rule"`
-}
-
-type Container struct {
-	Name     string                    `toml:"name"`
-	Auth     ImageAuth                 `toml:"auth"`
-	Command  []string                  `toml:"command"`
-	EnvRules []EnvironmentVariableRule `toml:"env_rule"`
-}
-
-type ImageAuth struct {
-	Username string `toml:"username"`
-	Password string `toml:"password"`
-}
-
-type Config struct {
-	AllowAll   bool        `toml:"allow_all"`
-	Containers []Container `toml:"container"`
-}
-
-func createOpenDoorPolicy() securitypolicy.SecurityPolicy {
-	return securitypolicy.SecurityPolicy{
-		AllowAll: true,
-	}
-}
-
-func createPolicyFromConfig(config Config) (securitypolicy.SecurityPolicy, error) {
-	p := securitypolicy.SecurityPolicy{
-		Containers: securitypolicy.Containers{
-			Elements: map[string]securitypolicy.Container{},
-		},
-	}
-
+func createPolicyFromConfig(config *securitypolicy.PolicyConfig) (*securitypolicy.SecurityPolicy, error) {
 	// Hardcode the pause container version and command. We still pull it
 	// to get the root hash and any environment variable rules we might need.
-	pause := Container{
-		Name:     "k8s.gcr.io/pause:3.1",
-		Command:  []string{"/pause"},
-		EnvRules: []EnvironmentVariableRule{}}
+	pause := securitypolicy.NewContainerConfig(
+		"k8s.gcr.io/pause:3.1",
+		[]string{"/pause"},
+		[]securitypolicy.EnvRule{},
+		securitypolicy.AuthConfig{},
+	)
 	config.Containers = append(config.Containers, pause)
 
-	for _, configContainer := range config.Containers {
+	var policyContainers []*securitypolicy.Container
+	for _, containerConfig := range config.Containers {
 		var imageOptions []remote.Option
 
-		if configContainer.Auth.Username != "" && configContainer.Auth.Password != "" {
+		if containerConfig.Auth.Username != "" && containerConfig.Auth.Password != "" {
 			auth := authn.Basic{
-				Username: configContainer.Auth.Username,
-				Password: configContainer.Auth.Password}
+				Username: containerConfig.Auth.Username,
+				Password: containerConfig.Auth.Password}
 			c, _ := auth.Authorization()
 			authOption := remote.WithAuth(authn.FromConfig(*c))
 			imageOptions = append(imageOptions, authOption)
 		}
 
-		// validate EnvRules
-		err := validateEnvRules(configContainer.EnvRules)
+		ref, err := name.ParseReference(containerConfig.ImageName)
 		if err != nil {
-			return p, err
-		}
-
-		command := convertCommand(configContainer.Command)
-		envRules := convertEnvironmentVariableRules(configContainer.EnvRules)
-		container := securitypolicy.Container{
-			Command:  command,
-			EnvRules: envRules,
-			Layers: securitypolicy.Layers{
-				Elements: map[string]string{},
-			},
-		}
-		ref, err := name.ParseReference(configContainer.Name)
-		if err != nil {
-			return p, fmt.Errorf("'%s' isn't a valid image name", configContainer.Name)
+			return nil, fmt.Errorf("'%s' isn't a valid image name", containerConfig.ImageName)
 		}
 		img, err := remote.Image(ref, imageOptions...)
 		if err != nil {
-			return p, fmt.Errorf("unable to fetch image '%s': %s", configContainer.Name, err.Error())
+			return nil, fmt.Errorf("unable to fetch image '%s': %s", containerConfig.ImageName, err.Error())
 		}
 
 		layers, err := img.Layers()
 		if err != nil {
-			return p, err
+			return nil, err
 		}
 
+		var layerHashes []string
 		for _, layer := range layers {
 			r, err := layer.Uncompressed()
 			if err != nil {
-				return p, err
+				return nil, err
 			}
 
 			hashString, err := tar2ext4.ConvertAndComputeRootDigest(r)
 			if err != nil {
-				return p, err
+				return nil, err
 			}
-			addLayer(&container.Layers, hashString)
+			layerHashes = append(layerHashes, hashString)
 		}
 
 		// add rules for all known environment variables from the configuration
 		// these are in addition to "other rules" from the policy definition file
 		imgConfig, err := img.ConfigFile()
 		if err != nil {
-			return p, err
+			return nil, err
 		}
+
+		envRules := containerConfig.EnvRules
 		for _, env := range imgConfig.Config.Env {
 			rule := securitypolicy.EnvRule{
 				Strategy: securitypolicy.EnvVarRuleString,
 				Rule:     env,
 			}
-
-			addEnvRule(&container.EnvRules, rule)
+			envRules = append(envRules, rule)
 		}
-
 		// cri adds TERM=xterm for all workload containers. we add to all containers
 		// to prevent any possible error
 		rule := securitypolicy.EnvRule{
 			Strategy: securitypolicy.EnvVarRuleString,
 			Rule:     "TERM=xterm",
 		}
+		envRules = append(envRules, rule)
 
-		addEnvRule(&container.EnvRules, rule)
-
-		addContainer(&p.Containers, container)
-	}
-
-	return p, nil
-}
-
-func validateEnvRules(rules []EnvironmentVariableRule) error {
-	for _, rule := range rules {
-		switch rule.Strategy {
-		case securitypolicy.EnvVarRuleRegex:
-			_, err := regexp.Compile(rule.Rule)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
-}
-
-func convertCommand(toml []string) securitypolicy.CommandArgs {
-	jsn := map[string]string{}
-
-	for i, arg := range toml {
-		jsn[strconv.Itoa(i)] = arg
-	}
-
-	return securitypolicy.CommandArgs{
-		Elements: jsn,
-	}
-}
-
-func convertEnvironmentVariableRules(toml []EnvironmentVariableRule) securitypolicy.EnvRules {
-	jsn := map[string]securitypolicy.EnvRule{}
-
-	for i, rule := range toml {
-		jsonRule := securitypolicy.EnvRule{
-			Strategy: rule.Strategy,
-			Rule:     rule.Rule,
+		container, err := securitypolicy.NewContainer(containerConfig.Command, layerHashes, envRules)
+		if err != nil {
+			return nil, err
 		}
-
-		jsn[strconv.Itoa(i)] = jsonRule
+		policyContainers = append(policyContainers, container)
 	}
 
-	return securitypolicy.EnvRules{
-		Elements: jsn,
-	}
-}
-
-func addContainer(containers *securitypolicy.Containers, container securitypolicy.Container) {
-	index := strconv.Itoa(len(containers.Elements))
-
-	containers.Elements[index] = container
-}
-
-func addLayer(layers *securitypolicy.Layers, layer string) {
-	index := strconv.Itoa(len(layers.Elements))
-
-	layers.Elements[index] = layer
-}
-
-func addEnvRule(rules *securitypolicy.EnvRules, rule securitypolicy.EnvRule) {
-	index := strconv.Itoa(len(rules.Elements))
-
-	rules.Elements[index] = rule
+	return securitypolicy.NewSecurityPolicy(false, policyContainers), nil
 }