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

Added RunTimeLimit option and runner code to strip profiles metadata #3697

Merged
merged 2 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var conf = config.Compliance{
TmpDir: os.Getenv("TMPDIR"),
ResultMessageLimit: 10000,
ControlResultsLimit: 50,
RunTimeLimit: 1.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have a comment on this to document the unit of time? min? seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will put a comment and will mention it in the runFlags description.

},
ElasticSearch: config.ElasticSearch{},
ElasticSearchSidecar: config.ElasticSearchSidecar{
Expand Down Expand Up @@ -124,6 +125,7 @@ func init() {
runCmd.Flags().StringVar(&conf.InspecAgent.RemoteInspecVersion, "remote-inspec-version", conf.InspecAgent.RemoteInspecVersion, "Option to specify the version of inspec to use for remote(e.g. AWS SSM) scan jobs")
runCmd.Flags().IntVar(&conf.InspecAgent.ResultMessageLimit, "result-message-limit", conf.InspecAgent.ResultMessageLimit, "A control result message that exceeds this character limit will be truncated")
runCmd.Flags().IntVar(&conf.InspecAgent.ControlResultsLimit, "control-results-limit", conf.InspecAgent.ControlResultsLimit, "The array of results per control will be truncated at this limit to avoid large reports that cannot be processed")
runCmd.Flags().Float32Var(&conf.InspecAgent.RunTimeLimit, "run-time-limit", conf.InspecAgent.RunTimeLimit, "Control results that have a `run_time` below this limit will be stripped of the `start_time` and `run_time` fields")

// Legacy Automate Headers/User Info
runCmd.Flags().StringVar(&conf.Delivery.Enterprise, "delivery-ent", conf.Delivery.Enterprise, "Automate Enterprise")
Expand Down
2 changes: 2 additions & 0 deletions components/compliance-service/compliance.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func initBits(ctx context.Context, conf *config.Compliance) (db *pgdb.DB, connFa

inspec.ResultMessageLimit = conf.InspecAgent.ResultMessageLimit
runner.ControlResultsLimit = conf.InspecAgent.ControlResultsLimit
runner.RunTimeLimit = conf.InspecAgent.RunTimeLimit

inspec.TmpDir = conf.InspecAgent.TmpDir
// Let's have something sensible if the temp dir is not specified
Expand Down Expand Up @@ -172,6 +173,7 @@ func serveGrpc(ctx context.Context, db *pgdb.DB, connFactory *secureconn.Factory
nodeManagerServiceClient := getManagerConnection(connFactory, conf.Manager.Endpoint)
ingesticESClient := ingestic.NewESClient(esClient)
ingesticESClient.InitializeStore(context.Background())
runner.ESClient = ingesticESClient

s := connFactory.NewServer(tracing.GlobalServerInterceptor())

Expand Down
1 change: 1 addition & 0 deletions components/compliance-service/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type InspecAgent struct {
RemoteInspecVersion string
ResultMessageLimit int
ControlResultsLimit int
RunTimeLimit float32
}

// Postgres specific options
Expand Down
75 changes: 73 additions & 2 deletions components/compliance-service/inspec-agent/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
ingest_inspec "github.com/chef/automate/api/interservice/compliance/ingest/events/inspec"
"github.com/chef/automate/api/interservice/compliance/ingest/ingest"
"github.com/chef/automate/api/interservice/compliance/jobs"
"github.com/chef/automate/components/compliance-service/ingest/ingestic"
"github.com/chef/automate/components/compliance-service/ingest/ingestic/mappings"
"github.com/chef/automate/components/compliance-service/inspec"
"github.com/chef/automate/components/compliance-service/inspec-agent/remote"
Expand All @@ -37,6 +38,12 @@ var ListenPort int = 2133
// ControlResultsLimit used for configuring inspec exec command, passed in via config flag
var ControlResultsLimit int = 50

// RunTimeLimit used for report post processing to reduce the size of the report.
var RunTimeLimit float32 = 1.0

// Set from compliance.go to call the ElasticSearch backend
var ESClient *ingestic.ESClient

var (
ScanJobWorkflowName = cereal.NewWorkflowName("scan-job-workflow")

Expand Down Expand Up @@ -531,9 +538,22 @@ func (t *InspecJobTask) reportIt(ctx context.Context, job *types.InspecJob, cont
logrus.Debugf("hand-over report to ingest service")

removeResults(reportID, report.Profiles, ControlResultsLimit)
// strip profile meta if missing from the backend

_, err := t.ingestClient.ProcessComplianceReport(ctx, &report)
allReportProfileIds := make([]string, len(report.Profiles))
for i, profile := range report.Profiles {
allReportProfileIds[i] = profile.Sha256
}
esProfilesMissingMeta, err := ESClient.ProfilesMissing(allReportProfileIds)
if err != nil {
return errors.Wrap(err, "Report profiles processing error")
}
profilesMissingMeta := make(map[string]struct{}, 0)
for _, profileId := range esProfilesMissingMeta {
profilesMissingMeta[profileId] = struct{}{}
}
stripProfilesMetadata(&report, profilesMissingMeta, RunTimeLimit)

_, err = t.ingestClient.ProcessComplianceReport(ctx, &report)
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

if err != nil {
return errors.Wrap(err, "Report processing error")
}
Expand Down Expand Up @@ -756,6 +776,8 @@ func doExec(job *types.InspecJob) (jsonBytes []byte, err *inspec.Error) {
return jsonBytes, nil
}

// The purpose of this function is to remove the results from a control once they
// exceed `limit`. This avoids large reports that cannot be ingested.
func removeResults(reportId string, profiles []*ingest_inspec.Profile, limit int) {
for _, profile := range profiles {
if profile.Controls != nil {
Expand Down Expand Up @@ -788,6 +810,55 @@ func removeResults(reportId string, profiles []*ingest_inspec.Profile, limit int
}
}

// The purpose of this function is to remove the profiles metadata (control title, desc, code, etc)
// for profiles that already exist in the Automate profiles index.
func stripProfilesMetadata(report *ingest_events_compliance_api.Report, missingProfiles map[string]struct{}, runTimeLimit float32) {
if report.Profiles == nil {
return
}
for _, profile := range report.Profiles {
// If the profile is not in the ones missing in the backend, we can proceed with the metadata removal
if _, ok := missingProfiles[profile.Sha256]; ok {
continue
}
// Profile 'Name' is a required property. By not sending it in the report, we make it clear to the ingestion backend that the profile metadata has been stripped from this profile in the report.
// Profile 'title' and 'version' are still kept for troubleshooting purposes in the backend.
profile.Name = ""
profile.Groups = nil
profile.CopyrightEmail = ""
profile.Copyright = ""
profile.Summary = ""
profile.Supports = nil
profile.License = ""
profile.Maintainer = ""
profile.Depends = nil
if profile.Controls == nil {
continue
}
for _, control := range profile.Controls {
control.Code = ""
control.Desc = ""
// TODO: Add here control.Descriptions when the field makes it into the structs
control.Impact = 0
control.Refs = nil
control.Tags = nil
control.Title = ""
control.SourceLocation = nil
control.WaiverData = nil
if control.Results == nil {
continue
}
for _, result := range control.Results {
if result.RunTime < runTimeLimit {
result.RunTime = 0
result.StartTime = ""
}
}
}
report.RunTimeLimit = runTimeLimit
}
}

func potentialTargetConfigs(job *types.InspecJob) []inspec.TargetConfig {
// GCP profile requires the project_id to be passed in as an attribute. Used the SubscriptionId to get this value from the database
if job.TargetConfig.Backend == "gcp" {
Expand Down
214 changes: 214 additions & 0 deletions components/compliance-service/inspec-agent/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package runner
import (
"testing"

ingest_events_compliance_api "github.com/chef/automate/api/interservice/compliance/ingest/events/compliance"
ingest_inspec "github.com/chef/automate/api/interservice/compliance/ingest/events/inspec"
"github.com/chef/automate/components/compliance-service/inspec"
"github.com/chef/automate/components/compliance-service/inspec-agent/types"
"github.com/golang/protobuf/ptypes/struct"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -197,3 +199,215 @@ func TestRemoveResults(t *testing.T) {
}, profiles)

}

func TestStripProfilesMetadata(t *testing.T) {
report := ingest_events_compliance_api.Report{
ReportUuid: "test1",
Profiles: []*ingest_inspec.Profile{
{
Name: "test-profile-1",
Sha256: "test-profile-1-sha256-id",
Version: "4.5.6",
Maintainer: "Testus",
Copyright: "Testus Biz",
CopyrightEmail: "Testus Biz",
Title: "Test Profile 1",
Depends: []*ingest_inspec.Dependency{
{
Name: "test",
},
},
Supports: []*ingest_inspec.Support{
{
Inspec: "2.3.4",
},
},
Controls: []*ingest_inspec.Control{
{
Id: "p1c1",
Title: "p1c1 title",
Impact: 1,
Code: "some code",
Desc: "some desc",
SourceLocation: &ingest_inspec.SourceLocation{
Ref: "123",
Line: 456,
},
Refs: []*structpb.Struct{},
Tags: &structpb.Struct{},
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 0.12345,
StartTime: "2020-05-15T18:37:19+01:00",
},
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 1.12345,
StartTime: "2020-05-15T18:37:20+01:00",
},
},
},
{
Id: "p1c2",
Title: "p1c2 title",
Impact: 1,
Code: "some code",
Desc: "some desc",
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "skipped",
RunTime: 0.42345,
StartTime: "2020-05-15T18:37:21+01:00",
},
},
},
},
},
{
Name: "test-profile-2",
Sha256: "test-profile-2-sha256-id",
Version: "4.5.6",
Maintainer: "Testus2",
Copyright: "Testus Biz2",
CopyrightEmail: "Testus Biz2",
Title: "Test Profile 2",
Depends: []*ingest_inspec.Dependency{
{
Name: "test2",
},
},
Supports: []*ingest_inspec.Support{
{
Inspec: "2.3.5",
},
},
Controls: []*ingest_inspec.Control{
{
Id: "p2c1",
Title: "p2c1 title",
Impact: 0.6,
Code: "some code",
Desc: "some desc",
SourceLocation: &ingest_inspec.SourceLocation{
Ref: "12",
Line: 45,
},
Refs: []*structpb.Struct{},
Tags: &structpb.Struct{},
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 0.12345,
StartTime: "2020-05-15T18:37:19+01:00",
},
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 1.12345,
StartTime: "2020-05-15T18:37:20+01:00",
},
},
},
},
},
},
}
var missingProfilesMap = map[string]struct{}{
"test-profile-2-sha256-id": {},
}

stripProfilesMetadata(&report, missingProfilesMap, 1.1)

expectedReport := ingest_events_compliance_api.Report{
ReportUuid: "test1",
Profiles: []*ingest_inspec.Profile{
{
Sha256: "test-profile-1-sha256-id",
Version: "4.5.6",
Title: "Test Profile 1",
Controls: []*ingest_inspec.Control{
{
Id: "p1c1",
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "passed",
},
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 1.12345,
StartTime: "2020-05-15T18:37:20+01:00",
},
},
},
{
Id: "p1c2",
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "skipped",
},
},
},
},
},
{
Name: "test-profile-2",
Sha256: "test-profile-2-sha256-id",
Version: "4.5.6",
Maintainer: "Testus2",
Copyright: "Testus Biz2",
CopyrightEmail: "Testus Biz2",
Title: "Test Profile 2",
Depends: []*ingest_inspec.Dependency{
{
Name: "test2",
},
},
Supports: []*ingest_inspec.Support{
{
Inspec: "2.3.5",
},
},
Controls: []*ingest_inspec.Control{
{
Id: "p2c1",
Title: "p2c1 title",
Impact: 0.6,
Code: "some code",
Desc: "some desc",
SourceLocation: &ingest_inspec.SourceLocation{
Ref: "12",
Line: 45,
},
Refs: []*structpb.Struct{},
Tags: &structpb.Struct{},
Results: []*ingest_inspec.Result{
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 0.12345,
StartTime: "2020-05-15T18:37:19+01:00",
},
{
Message: "Resource is as expected",
Status: "passed",
RunTime: 1.12345,
StartTime: "2020-05-15T18:37:20+01:00",
},
},
},
},
},
},
RunTimeLimit: 1.1,
}

assert.Equal(t, expectedReport, report)
}