Skip to content

Commit

Permalink
Merge branch 'main' into yakov.shapiro/MLOB-1652/update-codepwners
Browse files Browse the repository at this point in the history
  • Loading branch information
yshapiro-57 authored Oct 2, 2024
2 parents e473b7c + 8747c73 commit a5d3057
Show file tree
Hide file tree
Showing 23 changed files with 219 additions and 23 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@
/test/fakeintake/aggregator/servicediscoveryAggregator.go @DataDog/apm-onboarding @DataDog/universal-service-monitoring
/test/fakeintake/aggregator/servicediscoveryAggregator_test.go @DataDog/apm-onboarding @DataDog/universal-service-monitoring
/test/new-e2e/ @DataDog/agent-e2e-testing @DataDog/agent-devx-loops
/test/new-e2e/pkg/components/datadog-installer @DataDog/windows-agent
/test/new-e2e/test-infra-definition @DataDog/agent-devx-loops
/test/new-e2e/system-probe @DataDog/ebpf-platform
/test/new-e2e/scenarios/system-probe @DataDog/ebpf-platform
Expand Down
6 changes: 3 additions & 3 deletions .gitlab/e2e/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ new-e2e-installer:
- qa_installer_oci
- qa_agent_oci
variables:
TARGETS: ./tests/installer
TARGETS: ./tests/installer/unix
TEAM: fleet
FLEET_INSTALL_METHOD: "install_script"

Expand All @@ -349,7 +349,7 @@ new-e2e-installer-windows:
- export STABLE_INSTALLER_VERSION_PACKAGE=$(curl -sS https://hub.docker.com/v2/namespaces/datadog/repositories/installer-package/tags | jq -r '.results[] | .name' | sort | tail -n 2 | head -n 1)
- !reference [.new_e2e_template, before_script]
variables:
TARGETS: ./tests/installer
TARGETS: ./tests/installer/windows
TEAM: fleet
FLEET_INSTALL_METHOD: "windows"

Expand All @@ -369,7 +369,7 @@ new-e2e-installer-ansible:
- qa_installer_oci
- qa_agent_oci
variables:
TARGETS: ./tests/installer
TARGETS: ./tests/installer/unix
TEAM: fleet
FLEET_INSTALL_METHOD: "ansible"

Expand Down
28 changes: 28 additions & 0 deletions cmd/trace-agent/test/testsuite/traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,34 @@ func TestTraces(t *testing.T) {
})
})

t.Run("normalize, obfuscate, sqllexer", func(t *testing.T) {
if err := r.RunAgent([]byte("apm_config:\r\n features:[\"sqllexer\"]\r\n")); err != nil {
t.Fatal(err)
}
defer r.KillAgent()

p := testutil.GeneratePayload(1, &testutil.TraceConfig{
MinSpans: 4,
Keep: true,
}, nil)
for _, span := range p[0] {
span.Service = strings.Repeat("a", 200) // Too long
span.Name = strings.Repeat("b", 200) // Too long
}
p[0][0].Type = "sql"
p[0][0].Resource = "SELECT secret FROM users WHERE id = 123"
if err := r.Post(p); err != nil {
t.Fatal(err)
}
waitForTrace(t, &r, func(v *pb.AgentPayload) {
assert.Equal(t, "SELECT secret FROM users WHERE id = ?", v.TracerPayloads[0].Chunks[0].Spans[0].Resource)
for _, s := range v.TracerPayloads[0].Chunks[0].Spans {
assert.Len(t, s.Service, 100)
assert.Len(t, s.Name, 100)
}
})
})

t.Run("probabilistic", func(t *testing.T) {
if err := r.RunAgent([]byte("apm_config:\r\n probabilistic_sampler:\r\n enabled: true\r\n sampling_percentage: 100\r\n")); err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config_template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,7 @@ api_key:
## The list of items available under apm_config.features is not guaranteed to persist across versions;
## a feature may eventually be promoted to its own configuration option on the agent, or dropped entirely.
#
# features: ["error_rare_sample_tracer_drop","table_names","component2name","sql_cache","enable_otlp_compute_top_level_by_span_kind"]
# features: ["error_rare_sample_tracer_drop","table_names","component2name","sql_cache","sqllexer","enable_otlp_compute_top_level_by_span_kind"]

## @param additional_endpoints - object - optional
## @env DD_APM_ADDITIONAL_ENDPOINTS - object - optional
Expand Down
15 changes: 13 additions & 2 deletions pkg/fleet/installer/service/datadog_agent_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ package service
import (
"context"
"fmt"

"github.com/DataDog/datadog-agent/pkg/fleet/installer/repository"
"github.com/DataDog/datadog-agent/pkg/fleet/internal/cdn"
"github.com/DataDog/datadog-agent/pkg/fleet/internal/winregistry"
"github.com/DataDog/datadog-agent/pkg/util/log"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
)

Expand Down Expand Up @@ -103,8 +106,16 @@ func ConfigureAgent(_ context.Context, _ *cdn.CDN, _ *repository.Repositories) e
}

func installAgentPackage(target string, args []string) error {
// TODO: Need args here to restore DDAGENTUSER
err := msiexec(target, datadogAgent, "/i", args)
// Lookup stored Agent user and pass it to the Agent MSI
// TODO: bootstrap doesn't have a command-line agent user parameter yet,
// might need to update this when it does.
agentUser, err := winregistry.GetAgentUserName()
if err != nil {
return fmt.Errorf("failed to get Agent user: %w", err)
}
args = append(args, fmt.Sprintf("DDAGENTUSER_NAME=%s", agentUser))

err = msiexec(target, datadogAgent, "/i", args)
if err != nil {
return fmt.Errorf("failed to install Agent %s: %w", target, err)
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/fleet/internal/winregistry/winregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package winregistry

import (
"fmt"
"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"
"path/filepath"
Expand Down Expand Up @@ -42,3 +43,28 @@ func GetProgramDataDirForProduct(product string) (path string, err error) {
path = val
return
}

// GetAgentUserName returns the user name for the Agent, stored in the registry by the Installer MSI
func GetAgentUserName() (string, error) {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, "SOFTWARE\\Datadog\\Datadog Installer", registry.QUERY_VALUE)
if err != nil {
return "", err
}
defer k.Close()

user, _, err := k.GetStringValue("installedUser")
if err != nil {
return "", fmt.Errorf("could not read installedUser in registry: %w", err)
}

domain, _, err := k.GetStringValue("installedDomain")
if err != nil {
return "", fmt.Errorf("could not read installedDomain in registry: %w", err)
}

if domain != "" {
user = domain + `\` + user
}

return user, nil
}
9 changes: 9 additions & 0 deletions pkg/obfuscate/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,10 @@ func TestSQLTableFinderAndReplaceDigits(t *testing.T) {

func TestSQLQuantizer(t *testing.T) {
cases := []sqlTestCase{
{
`SELECT "table"."field" FROM "table" WHERE "table"."otherfield" = $? AND "table"."thirdfield" = $?;`,
`SELECT table . field FROM table WHERE table . otherfield = ? AND table . thirdfield = ?`,
},
{
"select * from users where id = 42",
"select * from users where id = ?",
Expand Down Expand Up @@ -2072,6 +2076,11 @@ func TestSQLLexerObfuscation(t *testing.T) {
query: "SELECT * FROM users WHERE id = 1",
expected: "SELECT * FROM users WHERE id = ?",
},
{
name: "dollar question paramerer",
query: `SELECT "table"."field" FROM "table" WHERE "table"."otherfield" = $? AND "table"."thirdfield" = $?;`,
expected: `SELECT "table"."field" FROM "table" WHERE "table"."otherfield" = $? AND "table"."thirdfield" = $?;`,
},
{
name: "simple query obfuscation with replace digits",
query: "SELECT * FROM users123 WHERE id = 1",
Expand Down
20 changes: 15 additions & 5 deletions pkg/obfuscate/sql_tokenizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,10 +473,15 @@ func (tkn *SQLTokenizer) Scan() (TokenKind, []byte) {
// modulo operator (e.g. 'id % 8')
return TokenKind(ch), tkn.bytes()
case '$':
if isDigit(tkn.lastChar) {
// TODO(gbbr): the first digit after $ does not necessarily guarantee
// that this isn't a dollar-quoted string constant. We might eventually
// want to cover for this use-case too (e.g. $1$some text$1$).
if isDigit(tkn.lastChar) || tkn.lastChar == '?' {
// TODO(knusbaum): Valid dollar quote tags start with alpha characters and contain no symbols.
// See: https://www.postgresql.org/docs/15/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
// See also: https://pgpedia.info/d/dollar-quoting.html instead.
//
// Instances of $[integer] or $? are prepared statement variables.
// We may eventually want to expand this to check for symbols other than numbers and '?',
// since other symbols are not valid dollar quote tags, but for now this covers prepared statement
// variables without exposing us to more risk of not obfuscating something than necessary.
return tkn.scanPreparedStatement('$')
}

Expand Down Expand Up @@ -678,11 +683,16 @@ func (tkn *SQLTokenizer) scanDollarQuotedString() (TokenKind, []byte) {

func (tkn *SQLTokenizer) scanPreparedStatement(_ rune) (TokenKind, []byte) {
// a prepared statement expect a digit identifier like $1
if !isDigit(tkn.lastChar) {
if !isDigit(tkn.lastChar) && tkn.lastChar != '?' {
tkn.setErr(`prepared statements must start with digits, got "%c" (%d)`, tkn.lastChar, tkn.lastChar)
return LexError, tkn.bytes()
}

if tkn.lastChar == '?' {
tkn.advance()
return PreparedStatement, tkn.bytes()
}

// scanNumber keeps the prefix rune intact.
// read numbers and return an error if any
token, buff := tkn.scanNumber(false)
Expand Down
8 changes: 8 additions & 0 deletions pkg/trace/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ type ObfuscationConfig struct {
CreditCards obfuscate.CreditCardsConfig `mapstructure:"credit_cards"`
}

func obfuscationMode(enabled bool) obfuscate.ObfuscationMode {
if enabled {
return obfuscate.ObfuscateOnly
}
return ""
}

// Export returns an obfuscate.Config matching o.
func (o *ObfuscationConfig) Export(conf *AgentConfig) obfuscate.Config {
return obfuscate.Config{
Expand All @@ -126,6 +133,7 @@ func (o *ObfuscationConfig) Export(conf *AgentConfig) obfuscate.Config {
KeepSQLAlias: conf.HasFeature("keep_sql_alias"),
DollarQuotedFunc: conf.HasFeature("dollar_quoted_func"),
Cache: conf.HasFeature("sql_cache"),
ObfuscationMode: obfuscationMode(conf.HasFeature("sqllexer")),
},
ES: o.ES,
OpenSearch: o.OpenSearch,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
features:
- |
APM: Add new 'sqllexer' feature flag for the Trace Agent, which enables
the sqllexer imprementation of the SQL Obfuscator.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
fixes:
- |
APM: Fix obfuscation of SQL queries containing non-numeric prepared statement variables.
20 changes: 17 additions & 3 deletions test/new-e2e/pkg/components/datadog-installer/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func (h *Component) Export(ctx *pulumi.Context, out *Output) error {

// Configuration represents the Windows NewDefender configuration
type Configuration struct {
URL string
URL string
AgentUser string
}

// Option is an optional function parameter type for Configuration options
Expand All @@ -57,6 +58,14 @@ func WithInstallURL(url string) func(*Configuration) error {
}
}

// WithAgentUser specifies the ddagentuser for the installation
func WithAgentUser(user string) func(*Configuration) error {
return func(p *Configuration) error {
p.AgentUser = user
return nil
}
}

// NewConfig creates a default config
func NewConfig(env config.Env, options ...Option) (*Configuration, error) {
if env.PipelineID() != "" {
Expand All @@ -79,14 +88,19 @@ func NewInstaller(e config.Env, host *remoteComp.Host, options ...Option) (*Comp
return nil, err
}

agentUserArg := ""
if params.AgentUser != "" {
agentUserArg = "DDAGENTUSER_NAME=" + params.AgentUser
}

hostInstaller, err := components.NewComponent(e, e.CommonNamer().ResourceName("datadog-installer"), func(comp *Component) error {
comp.namer = e.CommonNamer().WithPrefix("datadog-installer")
comp.Host = host

_, err = host.OS.Runner().Command(comp.namer.ResourceName("install"), &command.Args{
Create: pulumi.Sprintf(`
Exit (Start-Process -Wait msiexec -PassThru -ArgumentList '/qn /i %s').ExitCode
`, params.URL),
Exit (Start-Process -Wait msiexec -PassThru -ArgumentList '/qn /i %s %s').ExitCode
`, params.URL, agentUserArg),
Delete: pulumi.Sprintf(`
$installerList = Get-ItemProperty "HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*" | Where-Object {$_.DisplayName -like 'Datadog Installer'}
if (($installerList | measure).Count -ne 1) {
Expand Down
6 changes: 3 additions & 3 deletions test/new-e2e/tests/installer/windows/base_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e"
"github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments"
"github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner"
instlr "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer"
instlr "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer/unix"
suiteasserts "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer/windows/suite-assertions"
)

Expand Down Expand Up @@ -86,12 +86,12 @@ func (s *BaseInstallerSuite) StableAgentVersion() PackageVersion {

// SetupSuite checks that the environment variables are correctly setup for the test
func (s *BaseInstallerSuite) SetupSuite() {
s.BaseSuite.SetupSuite()

if instlr.GetInstallMethodFromEnv(s.T()) != instlr.InstallMethodWindows {
s.T().Skip("Skipping Windows-only tests as the install method isn't Windows")
}

s.BaseSuite.SetupSuite()

// TODO:FA-779
if s.Env().Environment.PipelineID() == "" && os.Getenv("DD_INSTALLER_MSI_URL") == "" {
s.FailNow("E2E_PIPELINE_ID env var is not set, this test requires this variable to be set to work")
Expand Down
11 changes: 6 additions & 5 deletions test/new-e2e/tests/installer/windows/datadog_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,19 @@ package installer

import (
"fmt"
"os"
"path"
"path/filepath"
"strings"

"github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments"
"github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client"
"github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/optional"
"github.com/DataDog/datadog-agent/test/new-e2e/tests/installer"
installer "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer/unix"
windowsCommon "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common"
"github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent/installers/v2"
"github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/pipeline"
e2eos "github.com/DataDog/test-infra-definitions/components/os"
"os"
"path"
"path/filepath"
"strings"
)

const (
Expand Down
Loading

0 comments on commit a5d3057

Please sign in to comment.