diogomonica / actuary

An actuary is a business professional who analyzes the financial consequences of risk.
78 stars 14 forks source link

Code repetition #5

Closed diogomonica closed 8 years ago

diogomonica commented 8 years ago

There is a ton of code repetition in our tests.

When things like this show up multiple times:

    for _, container := range containers {
        info, _ := client.ContainerInspect(container.ID)
        ports := info.NetworkSettings.Ports
        for _, port := range ports {
            for _, portmap := range port {
                hostPort, _ := strconv.Atoi(portmap.HostPort)
                if hostPort < 1024 {
                    badContainers = append(badContainers, container.ID)
                }
            }
        }
    }

It might mean that we can try to abstract a method that runs a closure over each container, for example. Let's think of ways of creating good helper functions, and reducing code duplication.

/cc @zuBux

zuBux commented 8 years ago

I 've been working on a method for ContainerList similar to this :

func (l *ContainerList) RunCheck(f func() bool) []string

which will help us reduce this type of code repetition.

zuBux commented 8 years ago

Here is an example:

func CheckAppArmor(t Target) (res Result) {
    res.Name = "5.1 Verify AppArmor Profile, if applicable"
    containers := t.Containers
    if !containers.Running() {
        res.Skip("No running containers")
        return
    }
    apparmor := func(c ContainerInfo) bool {
        if c.AppArmorProfile == "" {
            return false
        }
        return true
    }

    badContainers := t.Containers.RunCheck(apparmor)
    if len(badContainers) == 0 {
        res.Pass()
    } else {
        output := fmt.Sprintf("Containers with no AppArmor profile: %s",
            badContainers)
        res.Fail(output)
    }
    return
}

where RunCheck is :

func (l *ContainerList) RunCheck(f func(c ContainerInfo) bool) (c []string) {
    for _, container := range *l {
        if f(container.Info) == false {
            c = append(c, container.ID)
        }
    }
    return
}

This saves us from repeating the same code over and over. Any suggestions/improvements?

diogomonica commented 8 years ago

Overall it feels like we're going in the right direction.

This component is going to be repeated everywhere too:

    if len(badContainers) == 0 {
        res.Pass()
    } else {
        output := fmt.Sprintf("Containers with no AppArmor profile: %s",
            badContainers)
        res.Fail(output)
    }

Maybe a lambda that takes from the context the error message and the condition, and passes it to runCheck as arguments, and that if else is also put inside of runCheck?