Skip to content

Commit

Permalink
Adding in the remaining unit tests for constraints, filled out some T…
Browse files Browse the repository at this point in the history
…ODOs, renamed getObjectViolations -> getObjectResults (#273)

Co-authored-by: Tommy Barnes <thbarnes@microsoft.com>
  • Loading branch information
tbarnes94 and tbarnes94 authored Apr 30, 2024
1 parent 5c1d552 commit 96d4e34
Show file tree
Hide file tree
Showing 44 changed files with 1,219 additions and 819 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/integration-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ jobs:
elif command -v /c/Users/runneradmin/.local/bin/draft &> /dev/null
then
echo "install_dir=/c/Users/runneradmin/.local/bin/draft" >> $GITHUB_ENV
elif command -v /Users/runner/.local/bin/draft &> /dev/null
then
echo "install_dir=/Users/runner/.local/bin/draft" >> $GITHUB_ENV
else
echo "draft could not be found"
exit 1
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ WORKDIR /draft
RUN apk add gcc musl-dev python3-dev libffi-dev openssl-dev cargo make
RUN apk add py3-pip

RUN python3 -m venv az-cli-env
RUN python3 -m venv az-cli-env
RUN az-cli-env/bin/pip install --upgrade pip
RUN az-cli-env/bin/pip install --upgrade azure-cli
RUN az-cli-env/bin/az --version
Expand All @@ -18,4 +18,4 @@ COPY . ./
RUN make go-generate

RUN go mod vendor
ENTRYPOINT ["go"]
ENTRYPOINT ["go"]
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
·
<a href="https://github.com/Azure/draft/issues">Request Feature</a>

[![Draft Unit Tests](https://github.com/Azure/draft/actions/workflows/unit-tests.yml/badge.svg)](https://github.com/Azure/draft/actions/workflows/unit-tests.yml)
[![Draft Unit Tests](https://github.com/Azure/draft/actions/workflows/unit-tests.yml/badge.svg?branch=main)](https://github.com/Azure/draft/actions/workflows/unit-tests.yml?query=branch:main)
[![GoDoc](https://godoc.org/github.com/Azure/draft?status.svg)](https://godoc.org/github.com/Azure/draft)
[![Go Report Card](https://goreportcard.com/badge/github.com/Azure/draft)](https://goreportcard.com/report/github.com/Azure/draft)
[![CodeQL](https://github.com/Azure/draft/actions/workflows/codeql-analysis.yml/badge.svg)](https://github.com/Azure/draft/actions/workflows/codeql-analysis.yml)
[![Draft Linux Integrations](https://github.com/Azure/draft/actions/workflows/integration-linux.yml/badge.svg)](https://github.com/Azure/draft/actions/workflows/integration-linux.yml)
[![Draft Release & Publish](https://github.com/Azure/draft/actions/workflows/release-and-publish.yml/badge.svg)](https://github.com/Azure/draft/actions/workflows/release-and-publish.yml)
[![Go Report Card](https://goreportcard.com/badge/github.com/Azure/draft?branch=main)](https://goreportcard.com/report/github.com/Azure/draft)
[![CodeQL](https://github.com/Azure/draft/actions/workflows/codeql-analysis.yml/badge.svg?branch=main)](https://github.com/Azure/draft/actions/workflows/codeql-analysis.yml?query=branch:main)
[![Draft Linux Integrations](https://github.com/Azure/draft/actions/workflows/integration-linux.yml/badge.svg?branch=main)](https://github.com/Azure/draft/actions/workflows/integration-linux.yml?query=branch:main)
[![Draft Release & Publish](https://github.com/Azure/draft/actions/workflows/release-and-publish.yml/badge.svg?branch=main)](https://github.com/Azure/draft/actions/workflows/release-and-publish.yml?query=branch:main)
</p>
</div>

Expand Down
1 change: 1 addition & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ For more information, please visit the Draft Github page: https://github.com/Azu
logrus.SetOutput(&logger.OutputSplitter{})
logrus.SetFormatter(new(logger.CustomFormatter))
},
SilenceErrors: true,
}

// Execute adds all child commands to the root command and sets flags appropriately.
Expand Down
44 changes: 39 additions & 5 deletions cmd/setup-gh.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package cmd

import (
"context"
"errors"
"fmt"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/subscription/armsubscription"
"github.com/Azure/draft/pkg/cred"
"github.com/manifoldco/promptui"
msgraph "github.com/microsoftgraph/msgraph-sdk-go"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"strings"

"github.com/Azure/draft/pkg/providers"
"github.com/Azure/draft/pkg/spinner"
Expand All @@ -23,11 +29,32 @@ func newSetUpCmd() *cobra.Command {
Long: `This command will automate the Github OIDC setup process by creating an Azure Active Directory
application and service principle, and will configure that application to trust github.`,
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

azCred, err := cred.GetCred()
if err != nil {
return fmt.Errorf("getting credentials: %w", err)
}

client, err := armsubscription.NewTenantsClient(azCred, nil)
if err != nil {
return fmt.Errorf("creating tenants client: %w", err)
}

sc.AzClient.AzTenantClient = client

graphClient, err := createGraphClient(azCred)
if err != nil {
return fmt.Errorf("getting client: %w", err)
}

sc.AzClient.GraphClient = graphClient

fillSetUpConfig(sc)

s := spinner.CreateSpinner("--> Setting up Github OIDC...")
s.Start()
err := runProviderSetUp(sc, s)
err = runProviderSetUp(ctx, sc, s)
s.Stop()
if err != nil {
return err
Expand All @@ -49,6 +76,14 @@ application and service principle, and will configure that application to trust
return cmd
}

func createGraphClient(azCred *azidentity.DefaultAzureCredential) (providers.GraphClient, error) {
client, err := msgraph.NewGraphServiceClientWithCredentials(azCred, []string{cloud.AzurePublic.Services[cloud.ResourceManager].Endpoint + "/.default"})
if err != nil {
return nil, fmt.Errorf("creating graph service client: %w", err)
}
return &providers.GraphServiceClient{Client: client}, nil
}

func fillSetUpConfig(sc *providers.SetUpCmd) {
if sc.AppName == "" {
sc.AppName = getAppName()
Expand All @@ -72,11 +107,11 @@ func fillSetUpConfig(sc *providers.SetUpCmd) {
}
}

func runProviderSetUp(sc *providers.SetUpCmd, s spinner.Spinner) error {
func runProviderSetUp(ctx context.Context, sc *providers.SetUpCmd, s spinner.Spinner) error {
provider := strings.ToLower(sc.Provider)
if provider == "azure" {
// call azure provider logic
return providers.InitiateAzureOIDCFlow(sc, s)
return providers.InitiateAzureOIDCFlow(ctx, sc, s)

} else {
// call logic for user-submitted provider
Expand Down Expand Up @@ -204,5 +239,4 @@ func GetAzSubscriptionId(subIds []string) string {

func init() {
rootCmd.AddCommand(newSetUpCmd())

}
6 changes: 4 additions & 2 deletions cmd/setup-gh_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -10,6 +11,7 @@ import (
)

func TestSetUpConfig(t *testing.T) {
ctx := context.Background()
mockSetUpCmd := &providers.SetUpCmd{}
mockSetUpCmd.AppName = "testingSetUpCommand"
mockSetUpCmd.Provider = "Google"
Expand All @@ -20,7 +22,7 @@ func TestSetUpConfig(t *testing.T) {

fillSetUpConfig(mockSetUpCmd)

err := runProviderSetUp(mockSetUpCmd, s)
err := runProviderSetUp(ctx, mockSetUpCmd, s)

assert.True(t, err == nil)
}
}
77 changes: 9 additions & 68 deletions cmd/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ package cmd
import (
"context"
"fmt"
"io/fs"
"os"
"path"
"path/filepath"

"github.com/Azure/draft/pkg/safeguards"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"path"
)

type validateCmd struct {
Expand Down Expand Up @@ -44,81 +40,25 @@ func newValidateCmd() *cobra.Command {
return cmd
}

// isDirectory determines if a file represented by path is a directory or not
func isDirectory(path string) (bool, error) {
fileInfo, err := os.Stat(path)
if err != nil {
return false, err
}

return fileInfo.IsDir(), nil
}

// isYAML determines if a file is of the YAML extension or not
func isYAML(path string) bool {
return filepath.Ext(path) == ".yaml" || filepath.Ext(path) == ".yml"
}

// getManifests uses filepath.Walk to retrieve a list of the manifest files within the given manifest path
func getManifestFiles(p string) ([]safeguards.ManifestFile, error) {
var manifestFiles []safeguards.ManifestFile

noYamlFiles := true
err := filepath.Walk(p, func(walkPath string, info fs.FileInfo, err error) error {
manifest := safeguards.ManifestFile{}
// skip when walkPath is just given path and also a directory
if p == walkPath && info.IsDir() {
return nil
}

if err != nil {
return fmt.Errorf("error walking path %s with error: %w", walkPath, err)
}

if !info.IsDir() && info.Name() != "" && isYAML(walkPath) {
log.Debugf("%s is not a directory, appending to manifestFiles", info.Name())
noYamlFiles = false

manifest.Name = info.Name()
manifest.Path = walkPath
manifestFiles = append(manifestFiles, manifest)
} else if !isYAML(p) {
log.Debugf("%s is not a manifest file, skipping...", info.Name())
} else {
log.Debugf("%s is a directory, skipping...", info.Name())
}

return nil
})
if err != nil {
return nil, fmt.Errorf("could not walk directory: %w", err)
}
if noYamlFiles {
return nil, fmt.Errorf("no manifest files found within given path")
}

return manifestFiles, nil
}

// run is our entry point to GetManifestViolations
// run is our entry point to GetManifestResults
func (vc *validateCmd) run(c *cobra.Command) error {
if vc.manifestPath == "" {
return fmt.Errorf("path to the manifests cannot be empty")
}

ctx := context.Background()
isDir, err := isDirectory(vc.manifestPath)
isDir, err := safeguards.IsDirectory(vc.manifestPath)
if err != nil {
return fmt.Errorf("not a valid file or directory: %w", err)
}

var manifestFiles []safeguards.ManifestFile
if isDir {
manifestFiles, err = getManifestFiles(vc.manifestPath)
manifestFiles, err = safeguards.GetManifestFiles(vc.manifestPath)
if err != nil {
return err
}
} else if isYAML(vc.manifestPath) {
} else if safeguards.IsYAML(vc.manifestPath) {
manifestFiles = append(manifestFiles, safeguards.ManifestFile{
Name: path.Base(vc.manifestPath),
Path: vc.manifestPath,
Expand All @@ -132,15 +72,15 @@ func (vc *validateCmd) run(c *cobra.Command) error {
}

log.Debugf("validating manifests")
manifestViolations, err := safeguards.GetManifestViolations(ctx, manifestFiles)
manifestViolations, err := safeguards.GetManifestResults(ctx, manifestFiles)
if err != nil {
log.Errorf("validating safeguards: %s", err.Error())
return err
}

manifestViolationsFound := false
for _, v := range manifestViolations {
log.Printf("Analyzing %s for violations", v.Name)
manifestViolationsFound := false
// returning the full list of violations after each manifest is checked
for file, violations := range v.ObjectViolations {
log.Printf(" %s:", file)
Expand All @@ -154,8 +94,9 @@ func (vc *validateCmd) run(c *cobra.Command) error {
}
}

if len(manifestViolations) > 0 {
if manifestViolationsFound {
c.SilenceUsage = true
return fmt.Errorf("violations found")
} else {
log.Printf("✅ No violations found in \"%s\".", vc.manifestPath)
}
Expand Down
32 changes: 16 additions & 16 deletions cmd/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ func TestIsDirectory(t *testing.T) {
pathFalse := path.Join(testWd, "validate.go")
pathError := ""

isDir, err := isDirectory(pathTrue)
isDir, err := safeguards.IsDirectory(pathTrue)
assert.True(t, isDir)
assert.Nil(t, err)

isDir, err = isDirectory(pathFalse)
isDir, err = safeguards.IsDirectory(pathFalse)
assert.False(t, isDir)
assert.Nil(t, err)

isDir, err = isDirectory(pathError)
isDir, err = safeguards.IsDirectory(pathError)
assert.False(t, isDir)
assert.NotNil(t, err)
}
Expand All @@ -39,14 +39,14 @@ func TestIsYAML(t *testing.T) {
fileNotYaml, _ := filepath.Abs("../pkg/safeguards/tests/not-yaml/readme.md")
fileYaml, _ := filepath.Abs("../pkg/safeguards/tests/all/success/all-success-manifest-1.yaml")

assert.False(t, isYAML(fileNotYaml))
assert.True(t, isYAML(fileYaml))
assert.False(t, safeguards.IsYAML(fileNotYaml))
assert.True(t, safeguards.IsYAML(fileYaml))

manifestFiles, err := getManifestFiles(dirNotYaml)
manifestFiles, err := safeguards.GetManifestFiles(dirNotYaml)
assert.Nil(t, manifestFiles)
assert.NotNil(t, err)

manifestFiles, err = getManifestFiles(dirYaml)
manifestFiles, err = safeguards.GetManifestFiles(dirYaml)
assert.NotNil(t, manifestFiles)
assert.Nil(t, err)
}
Expand All @@ -62,35 +62,35 @@ func TestRunValidate(t *testing.T) {
var manifestFiles []safeguards.ManifestFile

// Scenario 1: empty manifest path should error
_, err := safeguards.GetManifestViolations(ctx, manifestFilesEmpty)
_, err := safeguards.GetManifestResults(ctx, manifestFilesEmpty)
assert.NotNil(t, err)

// Scenario 2a: manifest path leads to a directory of manifestFiles - expect success
manifestFiles, err = getManifestFiles(manifestPathDirectorySuccess)
manifestFiles, err = safeguards.GetManifestFiles(manifestPathDirectorySuccess)
assert.Nil(t, err)
v, err := safeguards.GetManifestViolations(ctx, manifestFiles)
v, err := safeguards.GetManifestResults(ctx, manifestFiles)
assert.Nil(t, err)
numViolations := countTestViolations(v)
assert.Equal(t, numViolations, 0)

// Scenario 2b: manifest path leads to a directory of manifestFiles - expect failure
manifestFiles, err = getManifestFiles(manifestPathDirectoryError)
manifestFiles, err = safeguards.GetManifestFiles(manifestPathDirectoryError)
assert.Nil(t, err)
v, err = safeguards.GetManifestViolations(ctx, manifestFiles)
v, err = safeguards.GetManifestResults(ctx, manifestFiles)
assert.Nil(t, err)
numViolations = countTestViolations(v)
assert.Greater(t, numViolations, 0)

// Scenario 3a: manifest path leads to one manifest file - expect success
manifestFiles, err = getManifestFiles(manifestPathFileSuccess)
v, err = safeguards.GetManifestViolations(ctx, manifestFiles)
manifestFiles, err = safeguards.GetManifestFiles(manifestPathFileSuccess)
v, err = safeguards.GetManifestResults(ctx, manifestFiles)
assert.Nil(t, err)
numViolations = countTestViolations(v)
assert.Equal(t, numViolations, 0)

// Scenario 3b: manifest path leads to one manifest file - expect failure
manifestFiles, err = getManifestFiles(manifestPathFileError)
v, err = safeguards.GetManifestViolations(ctx, manifestFiles)
manifestFiles, err = safeguards.GetManifestFiles(manifestPathFileError)
v, err = safeguards.GetManifestResults(ctx, manifestFiles)
assert.Nil(t, err)
numViolations = countTestViolations(v)
assert.Greater(t, numViolations, 0)
Expand Down
6 changes: 3 additions & 3 deletions cmd/validate_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package cmd

import "github.com/Azure/draft/pkg/safeguards"

func countTestViolations(violations []safeguards.ManifestViolation) int {
func countTestViolations(results []safeguards.ManifestResult) int {
numViolations := 0
for _, v := range violations {
numViolations += len(v.ObjectViolations)
for _, r := range results {
numViolations += len(r.ObjectViolations)
}

return numViolations
Expand Down
Loading

0 comments on commit 96d4e34

Please sign in to comment.