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

[aztfy enhancement proposal] -o option for specified output dir #15

Merged
merged 9 commits into from
Oct 15, 2021
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
4 changes: 3 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ type Config struct {
Logfile string `env:"AZTFY_LOGFILE" default:""`
Debug bool `env:"AZTFY_DEBUG" default:"false"`
MockClient bool `env:"AZTFY_MOCK_CLIENT" default:"false"`
OutputDir string // specified via CLI option
}

func NewConfig(rg string) (*Config, error) {
func NewConfig(rg string, outputDir string) (*Config, error) {
var cfg Config
if err := babyenv.Parse(&cfg); err != nil {
return nil, err
}
cfg.ResourceGroupName = rg
cfg.OutputDir = outputDir
return &cfg, nil
}
2 changes: 1 addition & 1 deletion internal/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ func NewMeta(cfg config.Config) (Meta, error) {
if cfg.MockClient {
return newMetaDummy(cfg.ResourceGroupName)
}
return newMetaImpl(cfg.ResourceGroupName)
return newMetaImpl(cfg.ResourceGroupName, cfg.OutputDir)
}
37 changes: 31 additions & 6 deletions internal/meta/meta_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -32,7 +33,7 @@ type MetaImpl struct {
armTemplate armtemplate.Template
}

func newMetaImpl(rg string) (Meta, error) {
func newMetaImpl(rg string, outputDir string) (Meta, error) {
ctx := context.TODO()

// Initialize the workspace
Expand All @@ -53,11 +54,35 @@ func newMetaImpl(rg string) (Meta, error) {
}

wsp := filepath.Join(rootDir, rg)
if err := os.RemoveAll(wsp); err != nil {
return nil, fmt.Errorf("removing existing workspace %q: %w", wsp, err)
}
if err := os.MkdirAll(wsp, 0755); err != nil {
return nil, fmt.Errorf("creating workspace %q: %w", wsp, err)

if outputDir != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we change the semantic of outputDir here a bit to make it the directory containing all the generated stuff, rather than a directory containing sub-directories where each of them corresponds to a resource group? If so, we need further ensure the outputDir is an empty directory before removing everything in there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@koudaiii Yep! Shall we further check wheter the wsp here is empty? If not, we shall error out, so that we don't accidentally remove anything for the user?

Copy link
Member Author

@koudaiii koudaiii Oct 14, 2021

Choose a reason for hiding this comment

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

Thank you for the review. Ok, We check that wsp is empty. If not, we shall error out because we don't accidentally remove anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

wsp = outputDir

// Ensure wsp is an empty directory
stat, err := os.Stat(wsp)
Copy link
Member Author

Choose a reason for hiding this comment

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

Using os.Stat is very nice. This makes more sense. https://pkg.go.dev/os#File.Stat

if os.IsNotExist(err) {
return nil, fmt.Errorf("the output directory %q doesn't exist", wsp)
}
if !stat.IsDir() {
return nil, fmt.Errorf("the output path %q is not a directory", wsp)
}

f, err := os.Open(wsp)
if err != nil {
return nil, err
}
_, err = f.Readdirnames(1) // Or f.Readdir(1)
f.Close()
if err != io.EOF {
return nil, fmt.Errorf("the output directory %q is not empty", wsp)
}
} else {
if err := os.RemoveAll(wsp); err != nil {
return nil, fmt.Errorf("removing existing workspace %q: %w", wsp, err)
}
if err := os.MkdirAll(wsp, 0755); err != nil {
return nil, fmt.Errorf("creating workspace %q: %w", wsp, err)
}
}

// Authentication
Expand Down
6 changes: 4 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import (
)

var (
flagVersion *bool
flagVersion *bool
flagOutputDir *string
)

func init() {
flagVersion = flag.Bool("v", false, "Print version")
flagOutputDir = flag.String("o", "", "Specify output dir. Default is a dir under the user cache dir, which is named after the resource group name")
}

const usage = `aztfy [option] <resource group name>
Expand All @@ -39,7 +41,7 @@ func main() {
os.Exit(1)
}

cfg, err := config.NewConfig(flag.Args()[0])
cfg, err := config.NewConfig(flag.Args()[0], *flagOutputDir)
if err != nil {
log.Fatal(err)
}
Expand Down