Skip to content
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
acdab09
Adapt codeowners check
jsoriano Jan 14, 2026
265c135
[citools] Add ListPackages and extend packageManifest struct
mrodm Apr 16, 2026
58f0fe5
[citools] Expose ReadPackageManifest and consolidate manifest logic
mrodm Apr 16, 2026
d9100b9
[codeowners] Refactor validatePackages to use citools.ListPackages
mrodm Apr 16, 2026
91f9db5
[codeowners] Add tests for validatePackages with nested packages
mrodm Apr 16, 2026
a4af05b
[magefile] Add ListPackages mage target
mrodm Apr 16, 2026
741448f
Test with a subset of packages
mrodm Apr 17, 2026
218f948
[buildkite] Fix package path/name misusage after ListPackages refactor
mrodm Apr 17, 2026
3d92f36
[citools] Export ManifestFileName and remove duplicate constant
mrodm Apr 17, 2026
bb2e60f
[buildkite] Fix leftover issues in test_one_package.sh
mrodm Apr 17, 2026
e2454f2
Ensure that package_name_manifest is executed in the package path
mrodm Apr 17, 2026
99bfc48
[buildkite] Add should_test_package helper to common.sh
mrodm Apr 17, 2026
9f03436
[buildkite] Refactor build_packages.sh to support nested packages
mrodm Apr 17, 2026
71e903e
Update temprarily build_packages to test it - to be reverted
mrodm Apr 17, 2026
293da9f
[buildkite] Fix mage not found in build_packages.sh
mrodm Apr 17, 2026
5651580
[codeowners] Fix infinite loop in findOwnerForFile on absolute paths
mrodm Apr 17, 2026
b794a1b
Update loop to find findOwnerForFile
mrodm Apr 17, 2026
ac65fe1
[buildkite] Replace PACKAGE_FOLDER_NAME with get_package_path in back…
mrodm Apr 20, 2026
d7c0536
[buildkite] Fix CODEOWNERS removal for nested packages in removeOther…
mrodm Apr 20, 2026
1f1ebbf
[buildkite] Fix get_package_path early exit under set -e
mrodm Apr 20, 2026
6708fcd
Use as source branch the current commit - to be removed
mrodm Apr 20, 2026
2fea205
Use list_all_directories to get all directories/packages
mrodm Apr 20, 2026
ca7362b
Add more test packages
mrodm Apr 20, 2026
6835aef
Attempt to get the current branch as a source
mrodm Apr 20, 2026
237d836
Create temporal branch
mrodm Apr 20, 2026
1e50707
Restore location of removeOtherPackages call
mrodm Apr 20, 2026
06a49b5
Move some packages under nested directories to test CI scripts
mrodm Apr 20, 2026
dc25416
Allow to list more packages
mrodm Apr 20, 2026
b9fcd33
Test codeowners
mrodm Apr 20, 2026
0e64850
Revert changes in CODEOWNERS file
mrodm Apr 20, 2026
96ff9bf
[github] Update sync-packages workflow to support nested package dire…
mrodm Apr 21, 2026
6d1992c
Use folder_name
mrodm Apr 21, 2026
d85c69c
[github] Use mage listPackages in sync-packages workflow
mrodm Apr 21, 2026
da33598
Test using just package folder name in github workflow
mrodm Apr 21, 2026
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
2 changes: 1 addition & 1 deletion .buildkite/scripts/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ teardown_test_package() {
}

list_all_directories() {
find . -maxdepth 1 -mindepth 1 -type d | xargs -I {} basename {} | sort
mage listPackages | grep -E '^packages/(elastic_package_registry|nginx)$'
}
Comment on lines 865 to 867
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Critical scripts/common.sh:831

list_all_directories() now hardcodes a grep filter that returns only two package names (elastic_package_registry and nginx). Since callers trigger_integrations_in_parallel.sh and test_integrations_with_serverless.sh iterate over this output to drive CI pipelines, all other packages are silently excluded from testing and publishing. If this restriction is intentional, consider documenting why only these two packages should run; otherwise, the filter should be removed to restore full package coverage.

 list_all_directories() {
-    mage listPackages | grep -E '^packages/(elastic_package_registry|nginx)$'
+    mage listPackages
 }
🤖 Copy this AI Prompt to have your agent fix this:
In file .buildkite/scripts/common.sh around lines 831-833:

`list_all_directories()` now hardcodes a grep filter that returns only two package names (`elastic_package_registry` and `nginx`). Since callers `trigger_integrations_in_parallel.sh` and `test_integrations_with_serverless.sh` iterate over this output to drive CI pipelines, all other packages are silently excluded from testing and publishing. If this restriction is intentional, consider documenting why only these two packages should run; otherwise, the filter should be removed to restore full package coverage.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This will be removed before merging the PR. This grep is there to test just a subset of packages instead of the ~400 packages.

Comment on lines 865 to 867
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 High scripts/common.sh:865

list_all_directories() at line 866 hardcodes a grep -E filter that restricts output to only 6 specific packages, silently skipping all other packages from the build/publish and serverless testing pipelines. This appears to be a development filter accidentally committed in place of the previous find implementation that listed all package directories. Consider reverting to the previous implementation or using a more appropriate mechanism for temporary filtering.

+# list all directories that are packages from the root of the repository
 list_all_directories() {
-    mage -d "${WORKSPACE}" listPackages | grep -E '^packages/(elastic_package_registry|nginx|rapid7_insightvm|sql_input|nginx/nginx|nginx/nginx_otel)$'
+    find . -maxdepth 1 -mindepth 1 -type d | xargs -I {} basename {} | sort
 }
🤖 Copy this AI Prompt to have your agent fix this:
In file .buildkite/scripts/common.sh around lines 865-867:

`list_all_directories()` at line 866 hardcodes a `grep -E` filter that restricts output to only 6 specific packages, silently skipping all other packages from the build/publish and serverless testing pipelines. This appears to be a development filter accidentally committed in place of the previous `find` implementation that listed all package directories. Consider reverting to the previous implementation or using a more appropriate mechanism for temporary filtering.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

All this filtering of packages is related to run different testing in CI pipelines. It will be removed that grep and keep the mage -d ${WORKSPACE} listPackages command before merging this PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for confirming. Would you like me to remove the grep filter and restore the command to just mage -d "${WORKSPACE}" listPackages, or will you handle this yourself?


check_package() {
Expand Down
2 changes: 1 addition & 1 deletion dev/citools/kibana.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

func KibanaConstraintPackage(path string) (*semver.Constraints, error) {
manifest, err := readPackageManifest(path)
manifest, err := ReadPackageManifest(path)
if err != nil {
return nil, fmt.Errorf("failed to read package manifest: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion dev/citools/logsdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func IsVersionLessThanLogsDBGA(version *semver.Version) bool {
}

func packageKibanaConstraint(path string) (*semver.Constraints, error) {
manifest, err := readPackageManifest(path)
manifest, err := ReadPackageManifest(path)
if err != nil {
return nil, err
}
Expand Down
21 changes: 17 additions & 4 deletions dev/citools/packagemanifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ package citools

import (
"fmt"
"slices"

"github.com/elastic/go-ucfg"
"github.com/elastic/go-ucfg/yaml"
)

var validTypes = []string{"integration", "input", "content"}

// kibanaConditions defines conditions for Kibana (e.g. required version).
type kibanaConditions struct {
Version string `config:"version" json:"version" yaml:"version"`
Expand All @@ -28,12 +31,22 @@ type conditions struct {
}

type packageManifest struct {
Name string `config:"name" json:"name" yaml:"name"`
License string `config:"license" json:"license" yaml:"license"`
Conditions conditions `config:"conditions" json:"conditions" yaml:"conditions"`
FormatVersion string `config:"format_version" json:"format_version" yaml:"format_version"`
Name string `config:"name" json:"name" yaml:"name"`
Type string `config:"type" json:"type" yaml:"type"`
Version string `config:"version" json:"version" yaml:"version"`
License string `config:"license" json:"license" yaml:"license"`
Conditions conditions `config:"conditions" json:"conditions" yaml:"conditions"`
}

func (m *packageManifest) IsValid() bool {
if m.FormatVersion == "" || m.Name == "" || m.Type == "" || m.Version == "" {
return false
}
return slices.Contains(validTypes, m.Type)
}

func readPackageManifest(path string) (*packageManifest, error) {
func ReadPackageManifest(path string) (*packageManifest, error) {
cfg, err := yaml.NewConfigWithFile(path, ucfg.PathSep("."))
if err != nil {
return nil, fmt.Errorf("reading file failed (path: %s): %w", path, err)
Expand Down
51 changes: 51 additions & 0 deletions dev/citools/packages.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package citools

import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"slices"
)

const manifestFileName = "manifest.yml"

// ListPackages returns the sorted paths of all packages found under dir.
func ListPackages(dir string) ([]string, error) {
var paths []string
err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !d.IsDir() {
return nil
}
manifestPath := filepath.Join(path, manifestFileName)
_, statErr := os.Stat(manifestPath)
if errors.Is(statErr, os.ErrNotExist) {
return nil
} else if statErr != nil {
return fmt.Errorf("error statting manifest %s: %w", manifestPath, statErr)
}
manifest, err := ReadPackageManifest(manifestPath)
if err != nil {
return fmt.Errorf("error reading manifest %s: %w", manifestPath, err)
}
if !manifest.IsValid() {
return nil
}
paths = append(paths, path)
// No need to look deeper once a package is found.
return filepath.SkipDir
})
if err != nil {
return nil, fmt.Errorf("error listing packages in %s: %w", dir, err)
}
slices.Sort(paths)
return paths, nil
}
2 changes: 1 addition & 1 deletion dev/citools/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

func packageSubscription(path string) (string, error) {
manifest, err := readPackageManifest(path)
manifest, err := ReadPackageManifest(path)
if err != nil {
return "", err
}
Expand Down
65 changes: 37 additions & 28 deletions dev/codeowners/codeowners.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
"bufio"
"fmt"
"os"
"path"
"path/filepath"
"strings"

"gopkg.in/yaml.v3"

"github.com/elastic/integrations/dev/citools"
)

const DefaultCodeownersPath = ".github/CODEOWNERS"
Expand Down Expand Up @@ -63,34 +64,26 @@ type githubOwners struct {
// data_streams, it checks that all data_streams are explicitly owned by a single owner. Such ownership
// sharing packages are identified by having at least one data_stream with explicit ownership in codeowners.
func validatePackages(codeowners *githubOwners, packagesDir string) error {
packageDirEntries, err := os.ReadDir(packagesDir)
paths, err := citools.ListPackages(packagesDir)
if err != nil {
return fmt.Errorf("error reading directory '%s': %w", packagesDir, err)
}

if len(packageDirEntries) == 0 {
if len(codeowners.owners) == 0 {
return nil
}
return fmt.Errorf("no packages found in %q", packagesDir)
return fmt.Errorf("error listing packages in %s: %w", packagesDir, err)
}

for _, packageDirEntry := range packageDirEntries {
packageName := packageDirEntry.Name()

packagePath := path.Join(packagesDir, packageName)

packageManifestPath := path.Join(packagePath, "manifest.yml")
err = codeowners.checkManifest(packageManifestPath)
for _, path := range paths {
err = codeowners.checkManifest(filepath.Join(path, "manifest.yml"))
if err != nil {
return fmt.Errorf("error checking manifest '%s': %w", packageManifestPath, err)
return fmt.Errorf("error checking manifest '%s': %w", path, err)
}

err = codeowners.checkDataStreams(packagePath)
err = codeowners.checkDataStreams(path)
if err != nil {
return fmt.Errorf("error checking data streams from '%s': %w", packagePath, err)
return fmt.Errorf("error checking data streams from '%s': %w", path, err)
}
}

if len(paths) == 0 {
if len(codeowners.owners) == 0 {
return nil
}
return fmt.Errorf("no packages found in %q", packagesDir)
}

return nil
Expand Down Expand Up @@ -170,10 +163,9 @@ func (codeowners *githubOwners) checkSingleField(field string) error {
}

func (codeowners *githubOwners) checkManifest(path string) error {
pkgDir := filepath.Dir(path)
owners, found := codeowners.owners["/"+pkgDir]
owners, found := codeowners.findOwnerForFile(path)
if !found {
return fmt.Errorf("there is no owner for %q in %q", pkgDir, codeowners.path)
return fmt.Errorf("there is no owner for %q in %q", filepath.Dir(path), codeowners.path)
}

content, err := os.ReadFile(path)
Expand Down Expand Up @@ -208,8 +200,25 @@ func (codeowners *githubOwners) checkManifest(path string) error {
return nil
}

func (codeowners *githubOwners) findOwnerForFile(path string) ([]string, bool) {
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
ownerDir := filepath.Dir(path)
for {
owners, found := codeowners.owners["/"+filepath.ToSlash(ownerDir)]
if found {
return owners, found
}

ownerDir = filepath.Dir(ownerDir)
if ownerDir == "." {
break
}
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
}

return nil, false
}

func (codeowners *githubOwners) checkDataStreams(packagePath string) error {
packageDataStreamsPath := path.Join(packagePath, "data_stream")
packageDataStreamsPath := filepath.Join(packagePath, "data_stream")
if _, err := os.Stat(packageDataStreamsPath); os.IsNotExist(err) {
// package doesn't have data_streams
return nil
Expand All @@ -229,8 +238,8 @@ func (codeowners *githubOwners) checkDataStreams(packagePath string) error {
var dataStreamsWithoutOwner []string
for _, dataStreamDirEntry := range dataStreamDirEntries {
dataStreamName := dataStreamDirEntry.Name()
dataStreamDir := path.Join(packageDataStreamsPath, dataStreamName)
dataStreamOwners, found := codeowners.owners["/"+dataStreamDir]
dataStreamDir := filepath.Join(packageDataStreamsPath, dataStreamName)
dataStreamOwners, found := codeowners.owners["/"+filepath.ToSlash(dataStreamDir)]
if !found {
dataStreamsWithoutOwner = append(dataStreamsWithoutOwner, dataStreamDir)
continue
Expand Down
25 changes: 25 additions & 0 deletions dev/codeowners/codeowners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,31 @@ func TestValidatePackages(t *testing.T) {
packageDir: "testdata/test_packages",
valid: true,
},
{
codeownersPath: "testdata/CODEOWNERS-nested-valid",
packageDir: "testdata/nested_packages",
valid: true,
},
{
codeownersPath: "testdata/CODEOWNERS-nested-missing-owner",
packageDir: "testdata/nested_packages",
valid: false,
},
{
codeownersPath: "testdata/CODEOWNERS-nested-category-owner",
packageDir: "testdata/nested_packages",
valid: true,
},
{
codeownersPath: "testdata/CODEOWNERS-nested-streams-valid",
packageDir: "testdata/nested_packages",
valid: true,
},
{
codeownersPath: "testdata/CODEOWNERS-nested-streams-missing-owners",
packageDir: "testdata/nested_packages",
valid: false,
},
}

for _, c := range cases {
Expand Down
2 changes: 2 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-nested-category-owner
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/testdata/nested_packages/package_top @elastic/integrations-developer-experience
/testdata/nested_packages/category @elastic/integrations-developer-experience
2 changes: 2 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-nested-missing-owner
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/testdata/nested_packages/package_top @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1 @elastic/integrations-developer-experience
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/testdata/nested_packages/package_top @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1 @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1/data_stream/stream_1 @pkoutsovasilis
/testdata/nested_packages/category/package_nested_2 @elastic/integrations-developer-experience
5 changes: 5 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-nested-streams-valid
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/testdata/nested_packages/package_top @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1 @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1/data_stream/stream_1 @pkoutsovasilis
/testdata/nested_packages/category/package_nested_1/data_stream/stream_2 @pkoutsovasilis
/testdata/nested_packages/category/package_nested_2 @elastic/integrations-developer-experience
3 changes: 3 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-nested-valid
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/testdata/nested_packages/package_top @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_1 @elastic/integrations-developer-experience
/testdata/nested_packages/category/package_nested_2 @elastic/integrations-developer-experience
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/devexp/manifest.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
format_version: 1.0.0
name: devexp
type: integration
version: 1.0.0
owner:
github: elastic/integrations-developer-experience
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
format_version: 1.0.0
name: package_nested_1
type: integration
version: 1.0.0
owner:
github: elastic/integrations-developer-experience
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
format_version: 1.0.0
name: package_nested_2
type: integration
version: 1.0.0
owner:
github: elastic/integrations-developer-experience
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
format_version: 1.0.0
name: package_top
type: integration
version: 1.0.0
owner:
github: elastic/integrations-developer-experience
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
format_version: 1.0.0
name: package_1
type: integration
version: 1.0.0
owner:
github: elastic/integrations-developer-experience
41 changes: 0 additions & 41 deletions dev/packagenames/manifest.go

This file was deleted.

Loading
Loading