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

Use buildplanner ImpactReport endpoint to show change summary. #3419

Merged
merged 8 commits into from
Aug 1, 2024
38 changes: 17 additions & 21 deletions internal/runbits/cves/cves.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/ActiveState/cli/internal/primer"
"github.com/ActiveState/cli/internal/rtutils/ptr"
"github.com/ActiveState/cli/pkg/buildplan"
"github.com/ActiveState/cli/pkg/platform/api/buildplanner/response"
vulnModel "github.com/ActiveState/cli/pkg/platform/api/vulnerabilities/model"
"github.com/ActiveState/cli/pkg/platform/api/vulnerabilities/request"
"github.com/ActiveState/cli/pkg/platform/model"
Expand All @@ -39,36 +40,31 @@ func NewCveReport(prime primeable) *CveReport {
return &CveReport{prime}
}

func (c *CveReport) Report(newBuildPlan *buildplan.BuildPlan, oldBuildPlan *buildplan.BuildPlan, names ...string) error {
changeset := newBuildPlan.DiffArtifacts(oldBuildPlan, false)
if c.shouldSkipReporting(changeset) {
func (c *CveReport) Report(report *response.ImpactReportResult, names ...string) error {
if !c.prime.Auth().Authenticated() {
logging.Debug("Skipping CVE reporting")
return nil
}

var ingredients []*request.Ingredient
for _, artifact := range changeset.Added {
for _, ing := range artifact.Ingredients {
ingredients = append(ingredients, &request.Ingredient{
Namespace: ing.Namespace,
Name: ing.Name,
Version: ing.Version,
})
for _, i := range report.Ingredients {
if i.After == nil {
continue // only care about additions or changes
}
}

for _, change := range changeset.Updated {
if !change.VersionsChanged() {
continue // For CVE reporting we only care about ingredient changes
if i.Before != nil && i.Before.Version == i.After.Version {
continue // only care about changes
}

for _, ing := range change.To.Ingredients {
ingredients = append(ingredients, &request.Ingredient{
Namespace: ing.Namespace,
Name: ing.Name,
Version: ing.Version,
})
}
ingredients = append(ingredients, &request.Ingredient{
Namespace: i.Namespace,
Name: i.Name,
Version: i.After.Version,
})
}

if len(ingredients) == 0 {
return nil
}

if len(names) == 0 {
Expand Down
49 changes: 27 additions & 22 deletions internal/runbits/dependencies/changesummary.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,49 @@ import (
"strconv"
"strings"

"github.com/go-openapi/strfmt"

"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/sliceutils"
"github.com/ActiveState/cli/pkg/buildplan"
"github.com/ActiveState/cli/pkg/platform/api/buildplanner/response"
)

// showUpdatedPackages specifies whether or not to include updated dependencies in the direct
// dependencies list, and whether or not to include updated dependencies when calculating indirect
// dependency numbers.
const showUpdatedPackages = true

// OutputChangeSummary looks over the given build plans, and computes and lists the additional
// dependencies being installed for the requested packages, if any.
func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan, oldBuildPlan *buildplan.BuildPlan) {
requested := newBuildPlan.RequestedArtifacts().ToIDMap()

func OutputChangeSummary(out output.Outputer, report *response.ImpactReportResult, buildPlan *buildplan.BuildPlan) {
// Process the impact report, looking for package additions or updates.
alreadyInstalledVersions := map[strfmt.UUID]string{}
addedString := []string{}
addedLocale := []string{}
dependencies := buildplan.Ingredients{}
directDependencies := buildplan.Ingredients{}
changeset := newBuildPlan.DiffArtifacts(oldBuildPlan, false)
for _, a := range changeset.Added {
if _, exists := requested[a.ArtifactID]; exists {
v := fmt.Sprintf("%s@%s", a.Name(), a.Version())
for _, i := range report.Ingredients {
if i.Before != nil {
alreadyInstalledVersions[strfmt.UUID(i.Before.IngredientID)] = i.Before.Version
}

if i.After == nil || !i.After.IsRequirement {
continue
}

if i.Before == nil {
Copy link
Member

Choose a reason for hiding this comment

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

May as well pair this up with the identical conditional on line 64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would, but they're not identical 😱

v := fmt.Sprintf("%s@%s", i.Name, i.After.Version)
addedString = append(addedLocale, v)
addedLocale = append(addedLocale, fmt.Sprintf("[ACTIONABLE]%s[/RESET]", v))
}

for _, i := range a.Ingredients {
dependencies = append(dependencies, i.RuntimeDependencies(true)...)
directDependencies = append(directDependencies, i.RuntimeDependencies(false)...)
for _, bpi := range buildPlan.Ingredients() {
if bpi.IngredientID != strfmt.UUID(i.After.IngredientID) {
continue
}
dependencies = append(dependencies, bpi.RuntimeDependencies(true)...)
directDependencies = append(directDependencies, bpi.RuntimeDependencies(false)...)
}
}

Expand All @@ -56,13 +67,7 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan,
return
}

// Process the existing runtime requirements into something we can easily compare against.
alreadyInstalled := buildplan.Artifacts{}
if oldBuildPlan != nil {
alreadyInstalled = oldBuildPlan.Artifacts()
}
oldRequirements := alreadyInstalled.Ingredients().ToIDMap()

// Output a summary of changes.
localeKey := "additional_dependencies"
if numIndirect > 0 {
localeKey = "additional_total_dependencies"
Expand Down Expand Up @@ -93,9 +98,9 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan,

item := fmt.Sprintf("[ACTIONABLE]%s@%s[/RESET]%s", // intentional omission of space before last %s
ingredient.Name, ingredient.Version, subdependencies)
oldVersion, exists := oldRequirements[ingredient.IngredientID]
if exists && ingredient.Version != "" && oldVersion.Version != ingredient.Version {
item = fmt.Sprintf("[ACTIONABLE]%s@%s[/RESET] → %s (%s)", oldVersion.Name, oldVersion.Version, item, locale.Tl("updated", "updated"))
oldVersion, exists := alreadyInstalledVersions[ingredient.IngredientID]
if exists && ingredient.Version != "" && oldVersion != ingredient.Version {
item = fmt.Sprintf("[ACTIONABLE]%s@%s[/RESET] → %s (%s)", ingredient.Name, oldVersion, item, locale.Tl("updated", "updated"))
}

out.Notice(fmt.Sprintf(" [DISABLED]%s[/RESET] %s", prefix, item))
Expand Down
33 changes: 19 additions & 14 deletions internal/runbits/runtime/requirements/requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/ActiveState/cli/internal/runbits/rationalize"
"github.com/ActiveState/cli/internal/runbits/runtime"
"github.com/ActiveState/cli/internal/runbits/runtime/trigger"
"github.com/ActiveState/cli/pkg/buildplan"
"github.com/ActiveState/cli/pkg/buildscript"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/api/buildplanner/response"
Expand Down Expand Up @@ -235,30 +234,36 @@ func (r *RequirementOperation) ExecuteRequirementOperation(ts *time.Time, requir

// Solve runtime
solveSpinner := output.StartSpinner(r.Output, locale.T("progress_solve_preruntime"), constants.TerminalAnimationInterval)
bpm := bpModel.NewBuildPlannerModel(r.Auth)
rtCommit, err := bpm.FetchCommit(commitID, r.Project.Owner(), r.Project.Name(), nil)
rtCommit, err := bp.FetchCommit(commitID, r.Project.Owner(), r.Project.Name(), nil)
if err != nil {
solveSpinner.Stop(locale.T("progress_fail"))
return errs.Wrap(err, "Failed to fetch build result")
}
solveSpinner.Stop(locale.T("progress_success"))

var oldBuildPlan *buildplan.BuildPlan
if rtCommit.ParentID != "" {
bpm := bpModel.NewBuildPlannerModel(r.Auth)
commit, err := bpm.FetchCommit(rtCommit.ParentID, r.Project.Owner(), r.Project.Name(), nil)
if err != nil {
return errs.Wrap(err, "Failed to fetch build result")
}
oldBuildPlan = commit.BuildPlan()
oldCommit, err := bp.FetchCommit(parentCommitID, r.Project.Owner(), r.Project.Name(), nil)
if err != nil {
solveSpinner.Stop(locale.T("progress_fail"))
return errs.Wrap(err, "Failed to fetch old build result")
}

// Fetch the impact report.
impactReport, err := bp.ImpactReport(&bpModel.ImpactReportParams{
Owner: r.prime.Project().Owner(),
Project: r.prime.Project().Name(),
Before: oldCommit.BuildScript(),
After: rtCommit.BuildScript(),
})
if err != nil {
return errs.Wrap(err, "Failed to fetch impact report")
}
solveSpinner.Stop(locale.T("progress_success"))

r.Output.Notice("") // blank line
dependencies.OutputChangeSummary(r.Output, rtCommit.BuildPlan(), oldBuildPlan)
dependencies.OutputChangeSummary(r.prime.Output(), impactReport, rtCommit.BuildPlan())

// Report CVEs
names := requirementNames(requirements...)
if err := cves.NewCveReport(r.prime).Report(rtCommit.BuildPlan(), oldBuildPlan, names...); err != nil {
if err := cves.NewCveReport(r.prime).Report(impactReport, names...); err != nil {
return errs.Wrap(err, "Could not report CVEs")
}

Expand Down
18 changes: 14 additions & 4 deletions internal/runners/commit/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,20 +147,30 @@ func (c *Commit) Run() (rerr error) {
}

// Get old buildplan.
commit, err := bp.FetchCommit(localCommitID, proj.Owner(), proj.Name(), nil)
oldCommit, err := bp.FetchCommit(localCommitID, proj.Owner(), proj.Name(), nil)
if err != nil {
return errs.Wrap(err, "Failed to fetch old commit")
}
oldBuildPlan := commit.BuildPlan()

// Fetch the impact report.
impactReport, err := bp.ImpactReport(&buildplanner.ImpactReportParams{
Owner: c.prime.Project().Owner(),
Project: c.prime.Project().Name(),
Before: oldCommit.BuildScript(),
After: rtCommit.BuildScript(),
})
if err != nil {
return errs.Wrap(err, "Failed to fetch impact report")
}

pgSolve.Stop(locale.T("progress_success"))
pgSolve = nil

// Output dependency list.
dependencies.OutputChangeSummary(out, rtCommit.BuildPlan(), oldBuildPlan)
dependencies.OutputChangeSummary(c.prime.Output(), impactReport, rtCommit.BuildPlan())

// Report CVEs.
if err := cves.NewCveReport(c.prime).Report(rtCommit.BuildPlan(), oldBuildPlan); err != nil {
if err := cves.NewCveReport(c.prime).Report(impactReport); err != nil {
return errs.Wrap(err, "Could not report CVEs")
}

Expand Down
23 changes: 19 additions & 4 deletions internal/runners/packages/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,37 @@ func (i *Import) Run(params *ImportRunParams) (rerr error) {
}

// Solve the runtime.
solveSpinner := output.StartSpinner(i.prime.Output(), locale.T("progress_solve_preruntime"), constants.TerminalAnimationInterval)
rtCommit, err := bp.FetchCommit(stagedCommitId, proj.Owner(), proj.Name(), nil)
if err != nil {
solveSpinner.Stop(locale.T("progress_fail"))
return errs.Wrap(err, "Failed to fetch build result for staged commit")
}

// Output change summary.
previousCommit, err := bp.FetchCommit(localCommitId, proj.Owner(), proj.Name(), nil)
if err != nil {
solveSpinner.Stop(locale.T("progress_fail"))
return errs.Wrap(err, "Failed to fetch build result for previous commit")
}
oldBuildPlan := previousCommit.BuildPlan()

// Fetch the impact report.
impactReport, err := bp.ImpactReport(&buildplanner.ImpactReportParams{
Owner: i.prime.Project().Owner(),
Project: i.prime.Project().Name(),
Before: previousCommit.BuildScript(),
After: rtCommit.BuildScript(),
})
if err != nil {
return errs.Wrap(err, "Failed to fetch impact report")
}
solveSpinner.Stop(locale.T("progress_success"))

// Output change summary.
out.Notice("") // blank line
dependencies.OutputChangeSummary(out, rtCommit.BuildPlan(), oldBuildPlan)
dependencies.OutputChangeSummary(i.prime.Output(), impactReport, rtCommit.BuildPlan())

// Report CVEs.
if err := cves.NewCveReport(i.prime).Report(rtCommit.BuildPlan(), oldBuildPlan); err != nil {
if err := cves.NewCveReport(i.prime).Report(impactReport); err != nil {
return errs.Wrap(err, "Could not report CVEs")
}

Expand Down
51 changes: 0 additions & 51 deletions pkg/buildplan/buildplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,57 +78,6 @@ func (b *BuildPlan) Ingredients(filters ...filterIngredient) Ingredients {
return b.ingredients.Filter(filters...)
}

func (b *BuildPlan) DiffArtifacts(oldBp *BuildPlan, requestedOnly bool) ArtifactChangeset {
// Basic outline of what needs to happen here:
// - add ArtifactID to the `Added` field if artifactID only appears in the the `new` buildplan
// - add ArtifactID to the `Removed` field if artifactID only appears in the the `old` buildplan
// - add ArtifactID to the `Updated` field if `ResolvedRequirements.feature` appears in both buildplans, but the resolved version has changed.

var new ArtifactNameMap
var old ArtifactNameMap

if requestedOnly {
new = b.RequestedArtifacts().ToNameMap()
old = oldBp.RequestedArtifacts().ToNameMap()
} else {
new = b.Artifacts().ToNameMap()
old = oldBp.Artifacts().ToNameMap()
}

var updated []ArtifactUpdate
var added []*Artifact
for name, artf := range new {
if artfOld, notNew := old[name]; notNew {
// The artifact name exists in both the old and new recipe, maybe it was updated though
if artfOld.ArtifactID == artf.ArtifactID {
continue
}
updated = append(updated, ArtifactUpdate{
From: artfOld,
To: artf,
})

} else {
// If it's not an update it is a new artifact
added = append(added, artf)
}
}

var removed []*Artifact
for name, artf := range old {
if _, noDiff := new[name]; noDiff {
continue
}
removed = append(removed, artf)
}

return ArtifactChangeset{
Added: added,
Removed: removed,
Updated: updated,
}
}

func (b *BuildPlan) Engine() types.BuildEngine {
buildEngine := types.Alternative
for _, s := range b.raw.Sources {
Expand Down
52 changes: 52 additions & 0 deletions pkg/platform/api/buildplanner/request/impactreport.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package request

func ImpactReport(organization, project string, beforeExpr, afterExpr []byte) *impactReport {
bp := &impactReport{map[string]interface{}{
"organization": organization,
"project": project,
"beforeExpr": string(beforeExpr),
"afterExpr": string(afterExpr),
}}

return bp
}

type impactReport struct {
vars map[string]interface{}
}

func (b *impactReport) Query() string {
return `
query ($organization: String!, $project: String!, $beforeExpr: BuildExpr!, $afterExpr: BuildExpr!) {
impactReport(
before: {organization: $organization, project: $project, buildExprOrCommit: {buildExpr: $beforeExpr}}
after: {organization: $organization, project: $project, buildExprOrCommit: {buildExpr: $afterExpr}}
) {
__typename
... on ImpactReport {
ingredients {
namespace
name
before {
ingredientID
version
isRequirement
}
after {
ingredientID
version
isRequirement
}
}
}
... on ImpactReportError {
message
}
}
}
`
}

func (b *impactReport) Vars() (map[string]interface{}, error) {
return b.vars, nil
}
Loading
Loading