Skip to content

Commit

Permalink
fix/chore: improve logging & validations, fix several issues, refacto…
Browse files Browse the repository at this point in the history 
…r codebase

This commit started with the goal of improving logging (skuid#220) to help aid with identifying when outcomes were not as expected due to the many issues currently present in the CLI and on the server-side.  During the course of improving the logging which required going through essentially every line of code in the codebase, many new issues were identified and fixes for nearly all of them implemented.  Additionally,validations of all steps in the CLI were improved to and logs emitted with the results.  Lastly, the code was significnatly refactored in many areas to improve code maintainability (skuid#230).

Due to the size of this commit, there is too much to explain in detail in a commit message.  See forthcoming comment in skuid#205 for full details of this commit.

Resolves skuid#59
Resolves skuid#165
Resolves skuid#199
Resolves skuid#218
Resolves skuid#220
Resolves skuid#221
Resolves skuid#222
Resolves skuid#223
Resolves skuid#224
Resolves skuid#228
Resolves skuid#230
Resolves skuid#231
  • Loading branch information
@techfg
techfg committed Oct 5, 2024
1 parent 7741c4b commit af20e1e
Show file tree
Hide file tree
Showing 51 changed files with 2,827 additions and 1,307 deletions.
2 changes: 1 addition & 1 deletion .unenv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ unset SKUID_PASSWORD
unset SKUID_ENTITIES

# Boolean
unset SKUID_DEBUG
unset SKUID_DIAG
unset SKUID_FILE_LOGGING
unset SKUID_IGNORE_SKUID_DB
unset SKUID_NO_CONSOLE_LOGGING
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ Where it makes sense, a mock may have a corresponding builder which is manually

To generate new mocks or update existing mocks:

1. Follow the instructions to install [mockery](https://vektra.github.io/mockery/latest/installation/).
1. Follow the instructions to install the latest version of [mockery](https://vektra.github.io/mockery/latest/installation/).
2. If updating a mock for an existing interface simply run `mockery` to re-generate all mocks.
3. If generating a new mock, update the `interfaces` list for the corresponding `package` (the package may need to be added) in [.mockery.yaml](.mockery.yaml) and run `mockery` to (re)generate all mocks.
113 changes: 54 additions & 59 deletions cmd/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ package cmd

import (
"fmt"
"path/filepath"
"time"

"github.com/gookit/color"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/skuid/skuid-cli/pkg"
Expand Down Expand Up @@ -36,7 +32,7 @@ func (c *deployCommander) GetCommand() *cobra.Command {
cmd := template.ToCommand(c.deploy)

cmdutil.AddAuthFlags(cmd, &c.authOpts)
cmdutil.AddStringFlag(cmd, &c.dir, flags.Dir)
cmdutil.AddValueFlag(cmd, &c.dir, flags.Dir)
cmdutil.AddStringFlag(cmd, &c.app, flags.App)
cmdutil.AddBoolFlag(cmd, &c.ignoreSkuidDb, flags.IgnoreSkuidDb)
cmdutil.AddSliceValueFlag(cmd, &c.entities, flags.Entities)
Expand All @@ -62,100 +58,99 @@ func NewCmdDeploy(factory *cmdutil.Factory) *cobra.Command {
return commander.GetCommand()
}

func (c *deployCommander) deploy(cmd *cobra.Command, _ []string) error {
processStart := time.Now()
fields := make(logrus.Fields)
fields["processStart"] = processStart
fields["processName"] = "deploy"
logging.WithFields(fields).Info(color.Green.Sprint("Starting Deploy"))

fields["host"] = c.authOpts.Host
fields["username"] = c.authOpts.Username
logging.WithFields(fields).Debug("Gathered credentials")

auth, err := pkg.Authorize(&c.authOpts)
// we don't need it anymore - very inelegant approach but at least it is something for now
// Clearing it here instead of in auth package which is the only place its accessed because the tests that exist
// for auth rely on package global variables so clearing in there would break those tests as they currently exist.
//
// TODO: Implement a solution for secure storage of the password while in memory and implement a proper one-time use
// approach assuming Skuid supports refresh tokens (see https://github.com/skuid/skuid-cli/issues/172)
// intentionally ignoring error since there is nothing we can do and we should fail entirely as a result
_ = c.authOpts.Password.Set("")
func (c *deployCommander) deploy(cmd *cobra.Command, args []string) (err error) {
message := fmt.Sprintf("Executing command %v entities to site %v from directory %v", logging.QuoteText(cmd.Name()), logging.ColorResource.Text(c.authOpts.Host), logging.ColorResource.QuoteText(c.dir))
fields := logging.Fields{
logging.CommandNameKey: cmd.Name(),
"host": c.authOpts.Host,
"username": c.authOpts.Username,
"sourceDirectory": c.dir,
"app": c.app,
"ignoreSkuidDb": c.ignoreSkuidDb,
"skipDataSources": c.skipDataSources,
"entites": logging.CSV(metadata.MetadataEntityPaths(c.entities).All()),
}
logger := logging.WithTracking("cmd.deploy", message, fields).StartTracking()
defer func() {
err = cmdutil.CheckError(cmd, err, recover())
logger = logger.FinishTracking(err)
err = cmdutil.HandleCommandResult(cmd, logger, err, fmt.Sprintf("Deployed site %v from %v", logging.ColorResource.Text(c.authOpts.Host), logging.ColorResource.QuoteText(c.dir)))
}()

auth, err := pkg.AuthorizeOnce(&c.authOpts)
if err != nil {
return err
}

fields["authorized"] = true
logging.WithFields(fields).Info("Authentication Successful")

var targetDirectory string
if targetDirectory, err = filepath.Abs(c.dir); err != nil {
return err
archiveFilter, entitiesToArchive := c.getArchiveFilter(logger)
options := pkg.DeployOptions{
ArchiveFilter: archiveFilter,
Auth: auth,
EntitiesToArchive: entitiesToArchive,
PlanFilter: c.getPlanFilter(logger),
SourceDirectory: c.dir,
}
fields["targetDirectory"] = targetDirectory

planFilter := c.getPlanFilter(fields)
archiveFilter, entitiesToArchive := c.getArchiveFilter(fields)
if err = pkg.Deploy(auth, targetDirectory, archiveFilter, entitiesToArchive, planFilter); err != nil {
if err := pkg.Deploy(options); err != nil {
return err
}

processFinish := time.Now()
fields["processFinish"] = processFinish
fields["processDuration"] = processFinish.Sub(processStart)
logging.WithFields(fields).Info(color.Green.Sprint("Finished Deploy"))

logger = logger.WithSuccess()
return nil
}

func (c *deployCommander) getPlanFilter(fields logrus.Fields) *pkg.NlxPlanFilter {
var filter *pkg.NlxPlanFilter = &pkg.NlxPlanFilter{}
func (c *deployCommander) getPlanFilter(logger *logging.Logger) *pkg.NlxPlanFilter {
var filter *pkg.NlxPlanFilter
initFilter := func() {
if filter == nil {
filter = &pkg.NlxPlanFilter{}
}
}

// filter by app name
if c.app != "" {
fields["appName"] = c.app
initFilter()
filter.AppName = c.app
logger.Debugf("Filtering deployment to app %v", logging.ColorFilter.QuoteText(c.app))
}

// filter by page name
// TODO: pages flag does not work as expected - remove completely or fix issues depending on https://github.com/skuid/skuid-cli/issues/147 & https://github.com/skuid/skuid-cli/issues/148
/*
if len(c.pages) > 0 {
fields["pages"] = c.pages
logEntry.Data["pages"] = c.pages
logEntry.Infof("Filtering retrieval to pages: %q", c.pages)
filter.PageNames = c.pages
}
*/

// ignore skuiddb
fields["ignoreSkuidDb"] = c.ignoreSkuidDb
filter.IgnoreSkuidDb = c.ignoreSkuidDb
if c.ignoreSkuidDb {
initFilter()
filter.IgnoreSkuidDb = c.ignoreSkuidDb
logger.Debug(logging.ColorFilter.Text("Ignoring any problematic Skuid Database metadata"))
}

return filter
}

// Resolve the pkg.ArchiveFilter to apply to the deployment
// Flags affecting the ArchiveFilter (e.g., Entities, SkipDataSources) should always be marked MutuallyExclusive
// so only one is eligible to be applied based on flags specified
func (c *deployCommander) getArchiveFilter(fields logrus.Fields) (pkg.ArchiveFilter, []metadata.MetadataEntity) {
func (c *deployCommander) getArchiveFilter(logger *logging.Logger) (pkg.ArchiveFilter, []metadata.MetadataEntity) {
if len(c.entities) > 0 {
// dedupe in case input contains same entity multiple times
uniqueEntities := metadata.UniqueEntities(c.entities)

var paths []string
for _, e := range uniqueEntities {
paths = append(paths, fmt.Sprintf("%q", e.Path))
}
fields["entities"] = paths
logging.WithFields(fields).Infof("Deploying entities: %v", paths)
return pkg.MetadataEntityArchiveFilter(c.entities), uniqueEntities
// paths := logging.CSV(seqs.Map(slices.Values(uniqueEntities), func(me metadata.MetadataEntity) string {
// return me.Path
// }))
paths := logging.CSV(metadata.MetadataEntityPaths(uniqueEntities).All())
logger.WithField("entityPaths", paths).Debugf("Filtering deployment to entities %v", logging.ColorFilter.Text(paths))
return pkg.MetadataEntityArchiveFilter(uniqueEntities), uniqueEntities
}

// skip datasources
// TODO: This can be removed once https://github.com/skuid/skuid-cli/issues/150 is resolved
if c.skipDataSources {
fields["skipDataSources"] = c.skipDataSources
logging.WithFields(fields).Info("Skipping deployment of all DataSources")
logger.Debugf("Skipping deployment of all %v", logging.ColorFilter.QuoteText("Data Sources"))
return pkg.MetadataTypeArchiveFilter([]metadata.MetadataType{metadata.MetadataTypeDataSources}), nil
}

Expand Down
148 changes: 47 additions & 101 deletions cmd/retrieve.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ package cmd

import (
"fmt"
"path/filepath"
"time"

"github.com/gookit/color"
"github.com/sirupsen/logrus"
"github.com/skuid/skuid-cli/pkg"
"github.com/skuid/skuid-cli/pkg/cmdutil"
"github.com/skuid/skuid-cli/pkg/flags"
Expand All @@ -33,7 +30,7 @@ func (c *retrieveCommander) GetCommand() *cobra.Command {
cmd := template.ToCommand(c.retrieve)

cmdutil.AddAuthFlags(cmd, &c.authOpts)
cmdutil.AddStringFlag(cmd, &c.dir, flags.Dir)
cmdutil.AddValueFlag(cmd, &c.dir, flags.Dir)
cmdutil.AddStringFlag(cmd, &c.app, flags.App)
cmdutil.AddValueFlag(cmd, &c.since, flags.Since)
cmdutil.AddBoolFlag(cmd, &c.noClean, flags.NoClean)
Expand All @@ -50,123 +47,72 @@ func NewCmdRetrieve(factory *cmdutil.Factory) *cobra.Command {
}

func (c *retrieveCommander) retrieve(cmd *cobra.Command, _ []string) (err error) {
fields := make(logrus.Fields)
start := time.Now()
fields["process"] = "retrieve"
fields["start"] = start
logging.WithFields(fields).Info(color.Green.Sprint("Starting Retrieve"))

fields["host"] = c.authOpts.Host
fields["username"] = c.authOpts.Username
logging.WithFields(fields).Debug("Credentials gathered")

auth, err := pkg.Authorize(&c.authOpts)
// we don't need it anymore - very inelegant approach but at least it is something for now
// Clearing it here instead of in auth package which is the only place its accessed because the tests that exist
// for auth rely on package global variables so clearing in there would break those tests as they currently exist.
//
// TODO: Implement a solution for secure storage of the password while in memory and implement a proper one-time use
// approach assuming Skuid supports refresh tokens (see https://github.com/skuid/skuid-cli/issues/172)
// intentionally ignoring error since there is nothing we can do and we should fail entirely as a result
_ = c.authOpts.Password.Set("")
message := fmt.Sprintf("Executing command %v entities from site %v to directory %v", logging.QuoteText(cmd.Name()), logging.ColorResource.Text(c.authOpts.Host), logging.ColorResource.QuoteText(c.dir))
fields := logging.Fields{
logging.CommandNameKey: cmd.Name(),
"host": c.authOpts.Host,
"username": c.authOpts.Username,
"targetDirectory": c.dir,
"app": c.app,
"since": c.since,
"noClean": c.noClean,
}
logger := logging.WithTracking("cmd.retrieve", message, fields).StartTracking()
defer func() {
err = cmdutil.CheckError(cmd, err, recover())
logger = logger.FinishTracking(err)
err = cmdutil.HandleCommandResult(cmd, logger, err, fmt.Sprintf("Retrieved site %v to %v", logging.ColorResource.Text(c.authOpts.Host), logging.ColorResource.QuoteText(c.dir)))
}()

auth, err := pkg.AuthorizeOnce(&c.authOpts)
if err != nil {
return
return err
}
options := pkg.RetrieveOptions{
Auth: auth,
NoClean: c.noClean,
PlanFilter: c.getPlanFilter(logger),
Since: c.since,
TargetDirectory: c.dir,
}
if err := pkg.Retrieve(options); err != nil {
return err
}

fields["authorized"] = true
logging.WithFields(fields).Info("Authentication Successful")
logger = logger.WithSuccess()
return nil
}

// we want the filter nil because it will be discarded without
// initialization
var filter *pkg.NlxPlanFilter = &pkg.NlxPlanFilter{}
func (c *retrieveCommander) getPlanFilter(logger *logging.Logger) *pkg.NlxPlanFilter {
var filter *pkg.NlxPlanFilter
initFilter := func() {
if filter == nil {
filter = &pkg.NlxPlanFilter{}
}
}

// filter by app name
if c.app != "" {
fields["appName"] = c.app
initFilter()
filter.AppName = c.app
logger.Debugf("Filtering retrieval to app %v", logging.ColorFilter.QuoteText(c.app))
}

// filter by page name
// TODO: pages flag does not work as expected - remove completely or fix issues depending on https://github.com/skuid/skuid-cli/issues/147 & https://github.com/skuid/skuid-cli/issues/148
/*
if len(c.pages) > 0 {
fields["pages"] = c.pages
initFilter()
logger.Infof("Filtering retrieval to pages: %q", c.pages)
filter.PageNames = c.pages
}
*/

var sinceStr string
if c.since != nil {
initFilter()
filter.Since = c.since.UTC()
sinceStr = filter.Since.Format(time.RFC3339)
fields["sinceUTC"] = sinceStr
logging.WithFields(fields).Info(fmt.Sprintf("retrieving metadata records updated since: %s", c.since.Format(time.RFC3339)))
}

logging.WithFields(fields).Info("Getting Retrieve Plan")

var plans pkg.NlxPlanPayload
if _, plans, err = pkg.GetRetrievePlan(auth, filter); err != nil {
return
}

logging.WithFields(fields).Info("Got Retrieve Plan")

// pliny and warden are supposed to give the since value back for the retrieve, but just in case...
if sinceStr != "" {
if plans.MetadataService.Since == "" {
plans.MetadataService.Since = sinceStr
}
if plans.CloudDataService != nil {
if plans.CloudDataService.Since == "" {
plans.CloudDataService.Since = sinceStr
}
}
logger.Debugf("Filtering retrieval to metadata records updated since %v", logging.ColorFilter.QuoteText(flags.FormatSince(c.since)))
}

var results []pkg.NlxRetrievalResult
if _, results, err = pkg.ExecuteRetrieval(auth, plans); err != nil {
return
}

fields["results"] = len(results)
fields["finished"] = time.Now()
fields["retrievalDuration"] = time.Since(start)

logging.WithFields(fields).Debugf("Received %v Results", color.Green.Sprint(len(results)))

var targetDirectory string
if targetDirectory, err = filepath.Abs(c.dir); err != nil {
return
}

fields["targetDirectory"] = targetDirectory
logging.WithFields(fields).Infof("Target Directory is %v", color.Cyan.Sprint(targetDirectory))

if !c.noClean {
logging.WithFields(fields).Infof("Cleaning target directory: %v", color.Cyan.Sprint(targetDirectory))
if err = pkg.ClearDirectories(targetDirectory); err != nil {
logging.Get().Errorf("Unable to clean target directory: %v", color.Cyan.Sprint(targetDirectory))
return
}
}

fields["writeStart"] = time.Now()

for _, v := range results {
if err = pkg.WriteResultsToDisk(
targetDirectory,
pkg.WritePayload{
PlanName: v.PlanName,
PlanData: v.Data,
},
); err != nil {
return
}
}

logging.Get().Infof("Finished Writing to %v", color.Cyan.Sprint(targetDirectory))
logging.WithFields(fields).Info(color.Green.Sprint("Finished Retrieve"))

return
return filter
}
Loading

0 comments on commit af20e1e

Please sign in to comment.