Skip to content

Commit

Permalink
Migrate secret pkg to component
Browse files Browse the repository at this point in the history
This change creates a `secrets.Component` that is implemented by an instance instead of global state. The instance class is `secretsimpl.secretResolver`

As part of this change, the agent's usage of secrets makes these changes:

* remove the methods config.NewAgentParamsWith{out}Secrets
  - instead we provide a secrets.NewEnabledParams() constructor
  - if not used, secrets is disabled by default
* remove the bool coreParams.ConfigLoadSecrets()
  - instead secretsParams.Enable is used
  - pass secretsParams as an fx dependency to the secretResolver
* change helper SetupConfigWithWarnings -> SetupConfigForTest
* remove the "secrets" build tag
* remove NewServerlessConfig's reference to WithConfigLoadSecret
  - serverless shouldn't use secrets, didn't have a build tag before
* remove configLoadSecrets from comp/core/config/params.go
  - let the secretsParam be the sole point of configuration instead
* stop almost all usage from the secrets component's global state

One hack is being added. The diagnose callbacks don't have access to components, so we need to get a reference to a global secrets component. We do this using a `GetInstance()` method, which returns a singleton instance protected by a mutex. This hack is only needed because changing the signature of diagnose is a larger project than can be done by this PR.
  • Loading branch information
dustmop committed Nov 21, 2023
1 parent 5ae7e92 commit a875603
Show file tree
Hide file tree
Showing 140 changed files with 899 additions and 671 deletions.
9 changes: 5 additions & 4 deletions cmd/agent/api/internal/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/DataDog/datadog-agent/cmd/agent/common/signals"
"github.com/DataDog/datadog-agent/cmd/agent/gui"
"github.com/DataDog/datadog-agent/comp/core/flare"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/comp/core/workloadmeta"
dogstatsdServer "github.com/DataDog/datadog-agent/comp/dogstatsd/server"
dogstatsddebug "github.com/DataDog/datadog-agent/comp/dogstatsd/serverDebug"
Expand All @@ -42,7 +43,6 @@ import (
"github.com/DataDog/datadog-agent/pkg/gohai"
"github.com/DataDog/datadog-agent/pkg/logs/diagnostic"
"github.com/DataDog/datadog-agent/pkg/metadata/inventories"
"github.com/DataDog/datadog-agent/pkg/secrets"
"github.com/DataDog/datadog-agent/pkg/status"
"github.com/DataDog/datadog-agent/pkg/status/health"
"github.com/DataDog/datadog-agent/pkg/tagger"
Expand All @@ -66,6 +66,7 @@ func SetupHandlers(
hostMetadata host.Component,
invAgent inventoryagent.Component,
invHost inventoryhost.Component,
secretResolver secrets.Component,
) *mux.Router {

r.HandleFunc("/version", common.GetVersion).Methods("GET")
Expand All @@ -89,7 +90,7 @@ func SetupHandlers(
r.HandleFunc("/workload-list", func(w http.ResponseWriter, r *http.Request) {
getWorkloadList(w, r, wmeta)
}).Methods("GET")
r.HandleFunc("/secrets", secretInfo).Methods("GET")
r.HandleFunc("/secrets", func(w http.ResponseWriter, r *http.Request) { secretInfo(w, r, secretResolver) }).Methods("GET")
r.HandleFunc("/metadata/{payload}", func(w http.ResponseWriter, r *http.Request) { metadataPayload(w, r, hostMetadata, invAgent, invHost) }).Methods("GET")
r.HandleFunc("/diagnose", func(w http.ResponseWriter, r *http.Request) { getDiagnose(w, r, senderManager) }).Methods("POST")

Expand Down Expand Up @@ -427,8 +428,8 @@ func getWorkloadList(w http.ResponseWriter, r *http.Request, wmeta workloadmeta.
w.Write(jsonDump)
}

func secretInfo(w http.ResponseWriter, r *http.Request) {
secrets.GetDebugInfo(w)
func secretInfo(w http.ResponseWriter, _ *http.Request, secretResolver secrets.Component) {
secretResolver.GetDebugInfo(w)
}

func metadataPayload(w http.ResponseWriter, r *http.Request, hostMetadataComp host.Component, invAgent inventoryagent.Component, invHost inventoryhost.Component) {
Expand Down
3 changes: 3 additions & 0 deletions cmd/agent/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/DataDog/datadog-agent/cmd/agent/api/internal/agent"
"github.com/DataDog/datadog-agent/cmd/agent/api/internal/check"
"github.com/DataDog/datadog-agent/comp/core/flare"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/comp/core/workloadmeta"
workloadmetaServer "github.com/DataDog/datadog-agent/comp/core/workloadmeta/server"
"github.com/DataDog/datadog-agent/comp/dogstatsd/replay"
Expand Down Expand Up @@ -64,6 +65,7 @@ func StartServer(
hostMetadata host.Component,
invAgent inventoryagent.Component,
invHost inventoryhost.Component,
secretResolver secrets.Component,
) error {
err := initializeTLS()
if err != nil {
Expand Down Expand Up @@ -146,6 +148,7 @@ func StartServer(
hostMetadata,
invAgent,
invHost,
secretResolver,
)))
mux.Handle("/check/", http.StripPrefix("/check", check.SetupHandlers(checkMux)))
mux.Handle("/", gwmux)
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type SubcommandFactory func(globalParams *GlobalParams) []*cobra.Command
// without secrets and logger disabled).
func GetDefaultCoreBundleParams(globalParams *GlobalParams) core.BundleParams {
return core.BundleParams{
ConfigParams: config.NewAgentParamsWithoutSecrets(globalParams.ConfFilePath),
ConfigParams: config.NewAgentParams(globalParams.ConfFilePath),
LogParams: log.ForOneShot(LoggerName, "off", true)}
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/common/autodiscovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"go.uber.org/atomic"
utilserror "k8s.io/apimachinery/pkg/util/errors"

"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/autodiscovery"
"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
"github.com/DataDog/datadog-agent/pkg/autodiscovery/providers"
Expand All @@ -37,8 +38,8 @@ var (
legacyProviders = []string{"kubelet", "container", "docker"}
)

func setupAutoDiscovery(confSearchPaths []string, metaScheduler *scheduler.MetaScheduler) *autodiscovery.AutoConfig {
ad := autodiscovery.NewAutoConfig(metaScheduler)
func setupAutoDiscovery(confSearchPaths []string, metaScheduler *scheduler.MetaScheduler, secretResolver secrets.Component) *autodiscovery.AutoConfig {
ad := autodiscovery.NewAutoConfig(metaScheduler, secretResolver)
providers.InitConfigFilesReader(confSearchPaths)
ad.AddConfigProvider(
providers.NewFileConfigProvider(),
Expand Down
25 changes: 8 additions & 17 deletions cmd/agent/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,21 @@ import (
"strings"

"github.com/DataDog/datadog-agent/cmd/agent/common/path"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
"github.com/DataDog/datadog-agent/pkg/config"
"github.com/DataDog/datadog-agent/pkg/config/model"
"github.com/DataDog/datadog-agent/pkg/config/settings"
"github.com/DataDog/datadog-agent/pkg/util/log"
"github.com/DataDog/datadog-agent/pkg/util/optional"

"github.com/DataDog/viper"
)

// SetupConfigWithWarnings fires up the configuration system and returns warnings if any.
func SetupConfigWithWarnings(confFilePath, configName string) (*config.Warnings, error) {
return setupConfig(config.Datadog, "datadog.yaml", confFilePath, configName, false, true, config.SystemProbe.GetEnvVars())
}

// SetupConfigWithoutSecrets fires up the configuration system without secrets support
func SetupConfigWithoutSecrets(confFilePath string, configName string) error {
_, err := setupConfig(config.Datadog, "datadog.yaml", confFilePath, configName, true, true, config.SystemProbe.GetEnvVars())
return err
}

func setupConfig(cfg config.Config, origin string, confFilePath string, configName string, withoutSecrets bool, failOnMissingFile bool, additionalKnownEnvVars []string) (*config.Warnings, error) {
if configName != "" {
cfg.SetConfigName(configName)
}
// SetupConfigForTest fires up the configuration system and returns warnings if any.
func SetupConfigForTest(confFilePath string) (*config.Warnings, error) {
cfg := config.Datadog
origin := "datadog.yaml"
// set the paths where a config file is expected
if len(confFilePath) != 0 {
// if the configuration file path was supplied on the command line,
Expand All @@ -49,10 +40,10 @@ func setupConfig(cfg config.Config, origin string, confFilePath string, configNa
}
cfg.AddConfigPath(path.DefaultConfPath)
// load the configuration
warnings, err := config.LoadDatadogCustom(cfg, origin, !withoutSecrets, nil)
warnings, err := config.LoadDatadogCustom(cfg, origin, optional.NewNoneOption[secrets.Component](), nil)
// If `!failOnMissingFile`, do not issue an error if we cannot find the default config file.
var e viper.ConfigFileNotFoundError
if err != nil && (failOnMissingFile || !errors.As(err, &e) || confFilePath != "") {
if err != nil && (!errors.As(err, &e) || confFilePath != "") {
// special-case permission-denied with a clearer error message
if errors.Is(err, fs.ErrPermission) {
if runtime.GOOS == "windows" {
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/common/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func ImportConfig(oldConfigDir string, newConfigDir string, force bool) error {

// setup the configuration system
config.Datadog.AddConfigPath(newConfigDir)
_, err = config.Load()
_, err = config.LoadWithoutSecret()
if err != nil {
return fmt.Errorf("unable to load Datadog config file: %s", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/agent/common/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"path/filepath"

"github.com/DataDog/datadog-agent/cmd/agent/common/path"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/comp/core/workloadmeta"
"github.com/DataDog/datadog-agent/pkg/aggregator/sender"
"github.com/DataDog/datadog-agent/pkg/autodiscovery/scheduler"
Expand Down Expand Up @@ -56,7 +57,7 @@ func GetWorkloadmetaInit() workloadmeta.InitHelper {

// LoadComponents configures several common Agent components:
// tagger, collector, scheduler and autodiscovery
func LoadComponents(ctx context.Context, senderManager sender.SenderManager, confdPath string) {
func LoadComponents(ctx context.Context, senderManager sender.SenderManager, secretResolver secrets.Component, confdPath string) {

confSearchPaths := []string{
confdPath,
Expand All @@ -71,7 +72,7 @@ func LoadComponents(ctx context.Context, senderManager sender.SenderManager, con
// No big concern here, but be sure to understand there is an implicit
// assumption about the initializtion of the tagger prior to being here.
// because of subscription to metadata store.
AC = setupAutoDiscovery(confSearchPaths, scheduler.NewMetaScheduler())
AC = setupAutoDiscovery(confSearchPaths, scheduler.NewMetaScheduler(), secretResolver)

sbomScanner, err := scanner.CreateGlobalScanner(config.Datadog)
if err != nil {
Expand All @@ -83,5 +84,4 @@ func LoadComponents(ctx context.Context, senderManager sender.SenderManager, con
// create the Collector instance and start all the components
// NOTICE: this will also setup the Python environment, if available
Coll = collector.NewCollector(senderManager, GetPythonPaths()...)

}
4 changes: 3 additions & 1 deletion cmd/agent/subcommands/configcheck/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/config"
"github.com/DataDog/datadog-agent/comp/core/log"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/flare"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)
Expand All @@ -44,7 +45,8 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command {
return fxutil.OneShot(run,
fx.Supply(cliParams),
fx.Supply(core.BundleParams{
ConfigParams: config.NewAgentParamsWithSecrets(globalParams.ConfFilePath),
ConfigParams: config.NewAgentParams(globalParams.ConfFilePath),
SecretParams: secrets.NewEnabledParams(),
LogParams: log.ForOneShot("CORE", "off", true)}),
core.Bundle,
)
Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/subcommands/configcheck/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)

Expand All @@ -20,8 +21,8 @@ func TestCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"configcheck", "-v"},
run,
func(cliParams *cliParams, coreParams core.BundleParams) {
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, true, cliParams.verbose)
require.Equal(t, true, coreParams.ConfigLoadSecrets())
require.Equal(t, true, secretParams.Enabled)
})
}
2 changes: 1 addition & 1 deletion cmd/agent/subcommands/diagnose/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command {
return fxutil.OneShot(cmdDiagnose,
fx.Supply(cliParams),
fx.Supply(core.BundleParams{
ConfigParams: config.NewAgentParamsWithoutSecrets(globalParams.ConfFilePath),
ConfigParams: config.NewAgentParams(globalParams.ConfFilePath),
LogParams: log.ForOneShot("CORE", "off", true)}),
core.Bundle,
diagnosesendermanagerimpl.Module,
Expand Down
21 changes: 11 additions & 10 deletions cmd/agent/subcommands/diagnose/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)

Expand All @@ -20,8 +21,8 @@ func TestDiagnoseCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"diagnose"},
cmdDiagnose,
func(cliParams *cliParams, coreParams core.BundleParams) {
require.Equal(t, false, coreParams.ConfigLoadSecrets())
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, false, secretParams.Enabled)
})
}

Expand All @@ -30,8 +31,8 @@ func TestShowMetadataV5Command(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"diagnose", "show-metadata", "v5"},
printPayload,
func(cliParams *cliParams, coreParams core.BundleParams) {
require.Equal(t, false, coreParams.ConfigLoadSecrets())
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, false, secretParams.Enabled)
require.Equal(t, "v5", cliParams.payloadName)
})
}
Expand All @@ -41,8 +42,8 @@ func TestShowMetadataGohaiCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"diagnose", "show-metadata", "gohai"},
printPayload,
func(cliParams *cliParams, coreParams core.BundleParams) {
require.Equal(t, false, coreParams.ConfigLoadSecrets())
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, false, secretParams.Enabled)
require.Equal(t, "gohai", cliParams.payloadName)
})
}
Expand All @@ -52,8 +53,8 @@ func TestShowMetadataInventoryCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"diagnose", "show-metadata", "inventory"},
printPayload,
func(cliParams *cliParams, coreParams core.BundleParams) {
require.Equal(t, false, coreParams.ConfigLoadSecrets())
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, false, secretParams.Enabled)
require.Equal(t, "inventory", cliParams.payloadName)
})
}
Expand All @@ -63,8 +64,8 @@ func TestShowMetadataInventoryAgentCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"diagnose", "show-metadata", "inventory-agent"},
printPayload,
func(cliParams *cliParams, coreParams core.BundleParams) {
require.Equal(t, false, coreParams.ConfigLoadSecrets())
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, false, secretParams.Enabled)
require.Equal(t, "inventory-agent", cliParams.payloadName)
})
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/subcommands/dogstatsdcapture/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)

Expand All @@ -21,9 +22,9 @@ func TestCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"dogstatsd-capture", "-d", "10s", "-z"},
dogstatsdCapture,
func(cliParams *cliParams, coreParams core.BundleParams) {
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, 10*time.Second, cliParams.dsdCaptureDuration)
require.True(t, cliParams.dsdCaptureCompressed)
require.Equal(t, false, coreParams.ConfigLoadSecrets())
require.Equal(t, false, secretParams.Enabled)
})
}
5 changes: 3 additions & 2 deletions cmd/agent/subcommands/dogstatsdreplay/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)

Expand All @@ -20,8 +21,8 @@ func TestCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"dogstatsd-replay", "-v"},
dogstatsdReplay,
func(cliParams *cliParams, coreParams core.BundleParams) {
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.True(t, cliParams.dsdVerboseReplay)
require.Equal(t, false, coreParams.ConfigLoadSecrets())
require.Equal(t, false, secretParams.Enabled)
})
}
5 changes: 3 additions & 2 deletions cmd/agent/subcommands/dogstatsdstats/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)

Expand All @@ -20,8 +21,8 @@ func TestCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"dogstatsd-stats", "--json"},
requestDogstatsdStats,
func(cliParams *cliParams, coreParams core.BundleParams) {
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.True(t, cliParams.jsonStatus)
require.Equal(t, false, coreParams.ConfigLoadSecrets())
require.Equal(t, false, secretParams.Enabled)
})
}
4 changes: 3 additions & 1 deletion cmd/agent/subcommands/flare/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/DataDog/datadog-agent/comp/core/flare"
"github.com/DataDog/datadog-agent/comp/core/flare/helpers"
"github.com/DataDog/datadog-agent/comp/core/log"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/comp/core/sysprobeconfig"
"github.com/DataDog/datadog-agent/comp/core/sysprobeconfig/sysprobeconfigimpl"
"github.com/DataDog/datadog-agent/comp/metadata/inventoryagent"
Expand Down Expand Up @@ -70,7 +71,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command {
Long: ``,
RunE: func(cmd *cobra.Command, args []string) error {
cliParams.args = args
config := config.NewAgentParamsWithSecrets(globalParams.ConfFilePath,
config := config.NewAgentParams(globalParams.ConfFilePath,
config.WithSecurityAgentConfigFilePaths([]string{
path.Join(commonpath.DefaultConfPath, "security-agent.yaml"),
}),
Expand All @@ -81,6 +82,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command {
fx.Supply(cliParams),
fx.Supply(core.BundleParams{
ConfigParams: config,
SecretParams: secrets.NewEnabledParams(),
SysprobeConfigParams: sysprobeconfigimpl.NewParams(sysprobeconfigimpl.WithSysProbeConfFilePath(globalParams.SysProbeConfFilePath)),
LogParams: log.ForOneShot(command.LoggerName, "off", false),
}),
Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/subcommands/flare/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/DataDog/datadog-agent/cmd/agent/command"
"github.com/DataDog/datadog-agent/comp/core"
"github.com/DataDog/datadog-agent/comp/core/flare"
"github.com/DataDog/datadog-agent/comp/core/secrets"
"github.com/DataDog/datadog-agent/pkg/config"
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
)
Expand Down Expand Up @@ -159,8 +160,8 @@ func TestCommand(t *testing.T) {
Commands(&command.GlobalParams{}),
[]string{"flare", "1234"},
makeFlare,
func(cliParams *cliParams, coreParams core.BundleParams) {
func(cliParams *cliParams, coreParams core.BundleParams, secretParams secrets.Params) {
require.Equal(t, []string{"1234"}, cliParams.args)
require.Equal(t, true, coreParams.ConfigLoadSecrets())
require.Equal(t, true, secretParams.Enabled)
})
}
Loading

0 comments on commit a875603

Please sign in to comment.