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

Prohibit the configuration of services within modules #6215

Merged
merged 1 commit into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ internal API changes are not present.
Main (unreleased)
-----------------

### Breaking changes

- Prohibit the configuration of services within modules. (@wildum)

### Features

- A new `discovery.process` component for discovering Linux OS processes on the current host. (@korniltsev)
Expand Down
7 changes: 7 additions & 0 deletions docs/sources/flow/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ Other release notes for the different {{< param "PRODUCT_ROOT_NAME" >}} variants
[release-notes-operator]: {{< relref "../operator/release-notes.md" >}}
{{% /admonition %}}

## v0.40

### Breaking change: Prohibit the configuration of services within modules.

Previously it was possible to configure the HTTP service via the [HTTP config block](https://grafana.com/docs/agent/v0.39/flow/reference/config-blocks/http/) inside of a module.
This functionality is now only available in the main configuration.

## v0.39

### Breaking change: `otelcol.receiver.prometheus` will drop all `otel_scope_info` metrics when converting them to OTLP
Expand Down
11 changes: 11 additions & 0 deletions pkg/flow/internal/controller/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,17 @@ func (l *Loader) populateServiceNodes(g *dag.Graph, serviceBlocks []*ast.BlockSt
// Now, assign blocks to services.
for _, block := range serviceBlocks {
blockID := BlockComponentID(block).String()

if l.isModule() {
mattdurham marked this conversation as resolved.
Show resolved Hide resolved
diags.Add(diag.Diagnostic{
Severity: diag.SeverityLevelError,
Message: fmt.Sprintf("service blocks not allowed inside a module: %q", blockID),
StartPos: ast.StartPos(block).Position(),
EndPos: ast.EndPos(block).Position(),
})
continue
}

node := g.GetByID(blockID).(*ServiceNode)

// Blocks assigned to services are reset to nil in the previous loop.
Expand Down
38 changes: 38 additions & 0 deletions pkg/flow/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"time"

"github.com/grafana/agent/component"
"github.com/grafana/agent/pkg/flow/internal/controller"
"github.com/grafana/agent/pkg/flow/internal/worker"
"github.com/grafana/agent/pkg/flow/logging"
"github.com/grafana/agent/service"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -43,6 +45,9 @@ const exportDummy = `
value = "bob"
}`

const serviceConfig = `
testservice {}`

func TestModule(t *testing.T) {
tt := []struct {
name string
Expand Down Expand Up @@ -72,6 +77,12 @@ func TestModule(t *testing.T) {
exportModuleContent: exportStringConfig,
expectedErrorContains: "tracing block not allowed inside a module",
},
{
name: "Service blocks not allowed in module config",
argumentModuleContent: argumentConfig + serviceConfig,
exportModuleContent: exportStringConfig,
expectedErrorContains: "service blocks not allowed inside a module: \"testservice\"",
},
{
name: "Argument not defined in module source",
argumentModuleContent: `argument "different_argument" {}`,
Expand Down Expand Up @@ -245,12 +256,19 @@ func testModuleControllerOptions(t *testing.T) *moduleControllerOptions {
s, err := logging.New(os.Stderr, logging.DefaultOptions)
require.NoError(t, err)

services := []service.Service{
&testService{},
}

serviceMap := controller.NewServiceMap(services)

return &moduleControllerOptions{
Logger: s,
DataPath: t.TempDir(),
Reg: prometheus.NewRegistry(),
ModuleRegistry: newModuleRegistry(),
WorkerPool: worker.NewFixedWorkerPool(1, 100),
ServiceMap: serviceMap,
}
}

Expand Down Expand Up @@ -307,3 +325,23 @@ func (t *testModule) Run(ctx context.Context) error {
func (t *testModule) Update(_ component.Arguments) error {
return nil
}

type testService struct{}

func (t *testService) Definition() service.Definition {
return service.Definition{
Name: "testservice",
}
}

func (t *testService) Run(ctx context.Context, host service.Host) error {
return nil
}

func (t *testService) Update(newConfig any) error {
return nil
}

func (t *testService) Data() any {
return nil
}