From 8a10e7198df4b87244c60b2624542caf28f936ce Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 11:46:42 +0200 Subject: [PATCH 1/6] =?UTF-8?q?Revert=20"adding=20exclude=20files=20+=20ot?= =?UTF-8?q?her=20changes=20to=20get=20registration=20to=20work=20for=20?= =?UTF-8?q?=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 2284539973478c40cafa044ac3563f5009a7e72b. --- .../dockercompose/chip_ingress_set/protos.go | 420 +++++------------- 1 file changed, 108 insertions(+), 312 deletions(-) diff --git a/framework/components/dockercompose/chip_ingress_set/protos.go b/framework/components/dockercompose/chip_ingress_set/protos.go index b068f6515..d7b367b6c 100644 --- a/framework/components/dockercompose/chip_ingress_set/protos.go +++ b/framework/components/dockercompose/chip_ingress_set/protos.go @@ -29,7 +29,6 @@ type ProtoSchemaSet struct { Ref string `toml:"ref"` // ref or tag or commit SHA Folders []string `toml:"folders"` // if not provided, all protos will be fetched, otherwise only protos in these folders will be fetched SubjectPrefix string `toml:"subject_prefix"` // optional prefix for subjects - ExcludeFiles []string `toml:"exclude_files"` // files to exclude from registration (e.g., ['workflows/v2/execution_status.proto']) } // SubjectNamingStrategyFn is a function that is used to determine the subject name for a given proto file in a given repo @@ -51,6 +50,7 @@ func validateRepoConfiguration(repoConfig ProtoSchemaSet) error { if repoConfig.Ref != "" { return errors.New("ref is not supported with local protos with 'file://' prefix") } + return nil } @@ -76,13 +76,9 @@ func DefaultRegisterAndFetchProtos(ctx context.Context, client *github.Client, p } func RegisterAndFetchProtos(ctx context.Context, client *github.Client, protoSchemaSets []ProtoSchemaSet, schemaRegistryURL string, repoToSubjectNamingStrategy RepositoryToSubjectNamingStrategyFn) error { - framework.L.Info().Msgf("Registering and fetching protos from %d repositories", len(protoSchemaSets)) + framework.L.Debug().Msgf("Registering and fetching protos from %d repositories", len(protoSchemaSets)) for _, protoSchemaSet := range protoSchemaSets { - framework.L.Debug().Msgf("Processing proto schema set: %s", protoSchemaSet.URI) - if len(protoSchemaSet.ExcludeFiles) > 0 { - framework.L.Debug().Msgf("Excluding files: %s", strings.Join(protoSchemaSet.ExcludeFiles, ", ")) - } if valErr := validateRepoConfiguration(protoSchemaSet); valErr != nil { return errors.Wrapf(valErr, "invalid repo configuration for schema set: %v", protoSchemaSet) } @@ -93,18 +89,22 @@ func RegisterAndFetchProtos(ctx context.Context, client *github.Client, protoSch return client } + var client *github.Client + if token := os.Getenv("GITHUB_TOKEN"); token != "" { ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}) tc := oauth2.NewClient(ctx, ts) - return github.NewClient(tc) + client = github.NewClient(tc) + } else { + framework.L.Warn().Msg("GITHUB_TOKEN is not set, using unauthenticated GitHub client. This may cause rate limiting issues when downloading proto files") + client = github.NewClient(nil) } - framework.L.Warn().Msg("GITHUB_TOKEN is not set, using unauthenticated GitHub client. This may cause rate limiting issues when downloading proto files") - return github.NewClient(nil) + return client } for _, protoSchemaSet := range protoSchemaSets { - protos, protosErr := fetchProtoFilesInFolders(ctx, ghClientFn, protoSchemaSet.URI, protoSchemaSet.Ref, protoSchemaSet.Folders, protoSchemaSet.ExcludeFiles) + protos, protosErr := fetchProtoFilesInFolders(ctx, ghClientFn, protoSchemaSet.URI, protoSchemaSet.Ref, protoSchemaSet.Folders) if protosErr != nil { return errors.Wrapf(protosErr, "failed to fetch protos from %s", protoSchemaSet.URI) } @@ -129,7 +129,7 @@ func RegisterAndFetchProtos(ctx context.Context, client *github.Client, protoSch subjects[proto.Path] = subjectMessage } - registerErr := registerAllWithTopologicalSorting(schemaRegistryURL, protoMap, subjects, protoSchemaSet.Folders) + registerErr := registerAllWithTopologicalSortingByTrial(schemaRegistryURL, protoMap, subjects) if registerErr != nil { return errors.Wrapf(registerErr, "failed to register protos from %s", protoSchemaSet.URI) } @@ -168,64 +168,46 @@ func extractPackageNameWithRegex(protoSrc string) (string, error) { return matches[1], nil } -// extractTopLevelMessageNamesWithRegex extracts top-level message and enum names from a proto file using regex. +// we use simple regex to extract top-level message names from a proto file +// so that we don't need to parse the proto file with a parser (which would require a lot of dependencies) func extractTopLevelMessageNamesWithRegex(protoSrc string) ([]string, error) { - // Extract message names - messageMatches := regexp.MustCompile(`(?m)^\s*message\s+(\w+)\s*{`).FindAllStringSubmatch(protoSrc, -1) + matches := regexp.MustCompile(`(?m)^\s*message\s+(\w+)\s*{`).FindAllStringSubmatch(protoSrc, -1) var names []string - for _, match := range messageMatches { - if len(match) >= 2 { - names = append(names, match[1]) - } - } - - // Extract enum names - enumMatches := regexp.MustCompile(`(?m)^\s*enum\s+(\w+)\s*{`).FindAllStringSubmatch(protoSrc, -1) - for _, match := range enumMatches { + for _, match := range matches { if len(match) >= 2 { names = append(names, match[1]) } } if len(names) == 0 { - return nil, fmt.Errorf("no message or enum names found in proto source") + return nil, fmt.Errorf("no message names found in %s", protoSrc) } return names, nil } -// extractImportStatements extracts import statements from a proto source file using regex. -func extractImportStatements(protoSrc string) []string { - matches := regexp.MustCompile(`(?m)^\s*import\s+"([^"]+)"\s*;`).FindAllStringSubmatch(protoSrc, -1) - var imports []string - for _, match := range matches { - if len(match) >= 2 { - imports = append(imports, match[1]) - } - } - return imports -} +// Fetches .proto files from a GitHub repo optionally scoped to specific folders. It is recommended to use `*github.Client` with auth token to avoid rate limiting. +func fetchProtoFilesInFolders(ctx context.Context, clientFn func() *github.Client, uri, ref string, folders []string) ([]protoFile, error) { + framework.L.Debug().Msgf("Fetching proto files from %s in folders: %s", uri, strings.Join(folders, ", ")) -// fetchProtoFilesInFolders fetches .proto files from a GitHub repo optionally scoped to specific folders. -// It is recommended to use `*github.Client` with auth token to avoid rate limiting. -func fetchProtoFilesInFolders(ctx context.Context, clientFn func() *github.Client, uri, ref string, folders []string, excludeFiles []string) ([]protoFile, error) { if strings.HasPrefix(uri, "file://") { - return fetchProtosFromFilesystem(uri, folders, excludeFiles) + return fetchProtosFromFilesystem(uri, folders) } parts := strings.Split(strings.TrimPrefix(uri, "https://"), "/") - return fetchProtosFromGithub(ctx, clientFn, parts[1], parts[2], ref, folders, excludeFiles) + + return fetchProtosFromGithub(ctx, clientFn, parts[1], parts[2], ref, folders) } -func fetchProtosFromGithub(ctx context.Context, clientFn func() *github.Client, owner, repository, ref string, folders []string, excludeFiles []string) ([]protoFile, error) { - cachedFiles, found, cacheErr := loadCachedProtoFiles(owner, repository, ref, folders, excludeFiles) +func fetchProtosFromGithub(ctx context.Context, clientFn func() *github.Client, owner, repository, ref string, folders []string) ([]protoFile, error) { + cachedFiles, found, cacheErr := loadCachedProtoFiles(owner, repository, ref, folders) + if cacheErr != nil { + framework.L.Warn().Msgf("Failed to load cached proto files for %s/%s at ref %s: %v", owner, repository, ref, cacheErr) + } if cacheErr == nil && found { framework.L.Debug().Msgf("Using cached proto files for %s/%s at ref %s", owner, repository, ref) return cachedFiles, nil } - if cacheErr != nil { - framework.L.Warn().Msgf("Failed to load cached proto files for %s/%s at ref %s: %v", owner, repository, ref, cacheErr) - } client := clientFn() var files []protoFile @@ -248,11 +230,13 @@ searchLoop: } // if folders are specified, check prefix match + var folderFound string if len(folders) > 0 { matched := false for _, folder := range folders { if strings.HasPrefix(*entry.Path, strings.TrimSuffix(folder, "/")+"/") { matched = true + folderFound = folder break } } @@ -261,25 +245,10 @@ searchLoop: } } - // if excludeFiles are specified, check if the file should be excluded - if len(excludeFiles) > 0 { - excluded := false - for _, exclude := range excludeFiles { - if strings.HasPrefix(*entry.Path, exclude) { - framework.L.Debug().Msgf("Excluding proto file %s (matches exclude pattern: %s)", *entry.Path, exclude) - excluded = true - break - } - } - if excluded { - continue searchLoop - } - } - rawURL := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", owner, repository, sha, *entry.Path) resp, respErr := http.Get(rawURL) if respErr != nil { - return nil, errors.Wrapf(respErr, "failed to fetch %s", *entry.Path) + return nil, errors.Wrapf(respErr, "failed tofetch %s", *entry.Path) } defer resp.Body.Close() @@ -292,19 +261,26 @@ searchLoop: return nil, errors.Wrapf(bodyErr, "failed to read body for %s", *entry.Path) } + // subtract the folder from the path if it was provided, because if it is imported by some other protos + // most probably it will be imported as a relative path, so we need to remove the folder from the path + protoPath := *entry.Path + if folderFound != "" { + protoPath = strings.TrimPrefix(protoPath, strings.TrimSuffix(folderFound, "/")+"/") + } + files = append(files, protoFile{ Name: filepath.Base(*entry.Path), - Path: *entry.Path, + Path: protoPath, Content: string(body), }) } + framework.L.Debug().Msgf("Fetched %d proto files from Github's %s/%s", len(files), owner, repository) + if len(files) == 0 { return nil, fmt.Errorf("no proto files found in %s/%s in folders %s", owner, repository, strings.Join(folders, ", ")) } - framework.L.Debug().Msgf("Fetched %d proto files from %s/%s", len(files), owner, repository) - saveErr := saveProtoFilesToCache(owner, repository, ref, files) if saveErr != nil { framework.L.Warn().Msgf("Failed to save proto files to cache for %s/%s at ref %s: %v", owner, repository, ref, saveErr) @@ -313,7 +289,7 @@ searchLoop: return files, nil } -func loadCachedProtoFiles(owner, repository, ref string, folders []string, excludeFiles []string) ([]protoFile, bool, error) { +func loadCachedProtoFiles(owner, repository, ref string, _ []string) ([]protoFile, bool, error) { cachePath, cacheErr := cacheFilePath(owner, repository, ref) if cacheErr != nil { return nil, false, errors.Wrapf(cacheErr, "failed to get cache file path for %s/%s at ref %s", owner, repository, ref) @@ -323,7 +299,7 @@ func loadCachedProtoFiles(owner, repository, ref string, folders []string, exclu return nil, false, nil // cache not found } - cachedFiles, cachedErr := fetchProtosFromFilesystem("file://"+cachePath, folders, excludeFiles) + cachedFiles, cachedErr := fetchProtosFromFilesystem("file://"+cachePath, []string{}) // ignore folders since, we already filtered them when fetching from GitHub if cachedErr != nil { return nil, false, errors.Wrapf(cachedErr, "failed to load cached proto files from %s", cachePath) } @@ -340,14 +316,15 @@ func saveProtoFilesToCache(owner, repository, ref string, files []protoFile) err for _, file := range files { path := filepath.Join(cachePath, file.Path) if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return errors.Wrapf(err, "failed to create directory for cache file %s", path) + return errors.Wrapf(err, "failed to create directory for cache file %s", cachePath) } if writeErr := os.WriteFile(path, []byte(file.Content), 0755); writeErr != nil { - return errors.Wrapf(writeErr, "failed to write cached proto file to %s", path) + return errors.Wrapf(writeErr, "failed to write cached proto files to %s", cachePath) } } framework.L.Debug().Msgf("Saved %d proto files to cache at %s", len(files), cachePath) + return nil } @@ -359,10 +336,10 @@ func cacheFilePath(owner, repository, ref string) (string, error) { return filepath.Join(homeDir, ".local", "share", "beholder", "protobufs", owner, repository, ref), nil } -func fetchProtosFromFilesystem(uri string, folders []string, excludeFiles []string) ([]protoFile, error) { +func fetchProtosFromFilesystem(uri string, folders []string) ([]protoFile, error) { var files []protoFile - protoDirPath := strings.TrimPrefix(uri, "file://") + protoDirPath := strings.TrimPrefix(uri, "file://") walkErr := filepath.Walk(protoDirPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err @@ -372,39 +349,24 @@ func fetchProtosFromFilesystem(uri string, folders []string, excludeFiles []stri return nil } - if !strings.HasSuffix(path, ".proto") { - return nil - } - - relativePath := strings.TrimPrefix(strings.TrimPrefix(path, protoDirPath), "/") - - // if folders are specified, check prefix match + var folderFound string if len(folders) > 0 { matched := false for _, folder := range folders { - if strings.HasPrefix(relativePath, folder) { + if strings.HasPrefix(strings.TrimPrefix(strings.TrimPrefix(path, protoDirPath), "/"), folder) { matched = true + folderFound = folder break } } + if !matched { return nil } } - // if excludeFiles are specified, check if the file should be excluded - if len(excludeFiles) > 0 { - excluded := false - for _, exclude := range excludeFiles { - if strings.HasPrefix(relativePath, exclude) { - framework.L.Debug().Msgf("Excluding proto file %s (matches exclude pattern: %s)", relativePath, exclude) - excluded = true - break - } - } - if excluded { - return nil - } + if !strings.HasSuffix(path, ".proto") { + return nil } content, contentErr := os.ReadFile(path) @@ -412,24 +374,32 @@ func fetchProtosFromFilesystem(uri string, folders []string, excludeFiles []stri return errors.Wrapf(contentErr, "failed to read file at %s", path) } + // subtract the folder from the path if it was provided, because if it is imported by some other protos + // most probably it will be imported as a relative path, so we need to remove the folder from the path + protoPath := strings.TrimPrefix(strings.TrimPrefix(path, protoDirPath), "/") + if folderFound != "" { + protoPath = strings.TrimPrefix(strings.TrimPrefix(protoPath, folderFound), strings.TrimSuffix(folderFound, "/")) + protoPath = strings.TrimPrefix(protoPath, "/") + } + files = append(files, protoFile{ Name: filepath.Base(path), - Path: relativePath, + Path: protoPath, Content: string(content), }) return nil }) - if walkErr != nil { return nil, errors.Wrapf(walkErr, "failed to walk through directory %s", protoDirPath) } + framework.L.Debug().Msgf("Fetched %d proto files from local %s", len(files), protoDirPath) + if len(files) == 0 { return nil, fmt.Errorf("no proto files found in '%s' in folders %s", protoDirPath, strings.Join(folders, ", ")) } - framework.L.Debug().Msgf("Fetched %d proto files from local %s", len(files), protoDirPath) return files, nil } @@ -452,126 +422,74 @@ type schemaStatus struct { Version int } -// registerAllWithTopologicalSorting registers protos in dependency order using topological sorting -func registerAllWithTopologicalSorting( +// registerAllWithTopologicalSortingByTrial tries to register protos that have not been registered yet, and if it fails, it tries again with a different order +// it keeps doing this until all protos are registered or it fails to register any more protos +func registerAllWithTopologicalSortingByTrial( schemaRegistryURL string, protoMap map[string]string, // path -> proto source subjectMap map[string]string, // path -> subject - folders []string, // folders configuration used to determine import prefix transformations ) error { - framework.L.Info().Msgf("Registering %d protobuf schemas", len(protoMap)) - - // Build dependency graph and sort topologically - dependencies, depErr := buildDependencyGraph(protoMap) - if depErr != nil { - return errors.Wrap(depErr, "failed to build dependency graph") - } - - sortedFiles, sortErr := topologicalSort(dependencies) - if sortErr != nil { - return errors.Wrap(sortErr, "failed to sort files topologically") - } - - framework.L.Debug().Msgf("Registration order (topologically sorted): %v", sortedFiles) - + framework.L.Info().Msgf("🔄 registering %d protobuf schemas", len(protoMap)) schemas := map[string]*schemaStatus{} for path, src := range protoMap { schemas[path] = &schemaStatus{Source: src} } - // Register files in topological order - for _, path := range sortedFiles { - schema, exists := schemas[path] - if !exists { - framework.L.Warn().Msgf("File %s not found in schemas map", path) - continue - } + refs := []map[string]any{} - if schema.Registered { - continue - } + for { + progress := false + failures := []string{} - subject, ok := subjectMap[path] - if !ok { - return fmt.Errorf("no subject found for %s", path) - } + for path, schema := range schemas { + if schema.Registered { + continue + } - // Determine which folder prefixes should be stripped based on configuration - prefixesToStrip := determineFolderPrefixesToStrip(folders) - - // Build references only for files that have dependencies - var fileRefs []map[string]any - if deps, hasDeps := dependencies[path]; hasDeps && len(deps) > 0 { - for _, dep := range deps { - if depSubject, depExists := subjectMap[dep]; depExists { - // The schema registry expects import names without the configured folder prefixes - // So if folders=["workflows"] and the import is "workflows/v1/metadata.proto", - // the name should be "v1/metadata.proto" - importName := stripFolderPrefix(dep, prefixesToStrip) - - fileRefs = append(fileRefs, map[string]any{ - "name": importName, - "subject": depSubject, - "version": 1, - }) - } + subject, ok := subjectMap[path] + if !ok { + failures = append(failures, fmt.Sprintf("%s: no subject found", path)) + continue + } + + singleProtoFailures := []error{} + framework.L.Debug().Msgf("🔄 registering %s as %s", path, subject) + _, registerErr := registerSingleProto(schemaRegistryURL, subject, schema.Source, refs) + if registerErr != nil { + failures = append(failures, fmt.Sprintf("%s: %v", path, registerErr)) + singleProtoFailures = append(singleProtoFailures, registerErr) + continue } - } - // Check if schema is already registered - if existingID, exists := checkSchemaExists(schemaRegistryURL, subject); exists { - framework.L.Debug().Msgf("Schema %s already exists with ID %d, skipping registration", subject, existingID) schema.Registered = true - schema.Version = existingID - continue - } + schema.Version = 1 + refs = append(refs, map[string]any{ + "name": path, + "subject": subject, + "version": 1, + }) - // The schema registry expects import statements without the configured folder prefixes - // Transform the schema content to remove these prefixes from import statements - modifiedSchema := transformSchemaContent(schema.Source, prefixesToStrip) + framework.L.Info().Msgf("✔ registered: %s as %s", path, subject) - _, registerErr := registerSingleProto(schemaRegistryURL, subject, modifiedSchema, fileRefs) - if registerErr != nil { - return errors.Wrapf(registerErr, "failed to register %s as %s", path, subject) + progress = true } - schema.Registered = true - schema.Version = 1 - - framework.L.Info().Msgf("✔ Registered: %s as %s", path, subject) + if !progress { + if len(failures) > 0 { + framework.L.Error().Msg("❌ Failed to register remaining schemas:") + for _, msg := range failures { + framework.L.Error().Msg(" " + msg) + } + return fmt.Errorf("unable to register %d schemas", len(failures)) + } + break + } } framework.L.Info().Msgf("✅ Successfully registered %d schemas", len(protoMap)) return nil } -// checkSchemaExists checks if a schema already exists in the registry -func checkSchemaExists(registryURL, subject string) (int, bool) { - url := fmt.Sprintf("%s/subjects/%s/versions", registryURL, subject) - - resp, err := http.Get(url) - if err != nil { - framework.L.Debug().Msgf("Failed to check schema existence for %s: %v", subject, err) - return 0, false - } - defer resp.Body.Close() - - if resp.StatusCode == 200 { - var versions []struct { - ID int `json:"id"` - } - if err := json.NewDecoder(resp.Body).Decode(&versions); err != nil { - framework.L.Debug().Msgf("Failed to decode versions for %s: %v", subject, err) - return 0, false - } - if len(versions) > 0 { - return versions[len(versions)-1].ID, true - } - } - - return 0, false -} - func registerSingleProto( registryURL, subject, schemaSrc string, references []map[string]any, @@ -615,128 +533,6 @@ func registerSingleProto( } framework.L.Debug().Msgf("Registered schema %s with ID %d", subject, result.ID) - return result.ID, nil -} - -// determineFolderPrefixesToStrip determines which folder prefixes should be stripped from import paths -// based on the folders configuration. The schema registry expects import names to be relative to the -// configured folders, so we strip these prefixes to make imports work correctly. -func determineFolderPrefixesToStrip(folders []string) []string { - var prefixes []string - for _, folder := range folders { - // Ensure folder ends with / for prefix matching - prefix := strings.TrimSuffix(folder, "/") + "/" - prefixes = append(prefixes, prefix) - } - return prefixes -} - -// stripFolderPrefix removes any configured folder prefixes from the given path -func stripFolderPrefix(path string, prefixes []string) string { - for _, prefix := range prefixes { - if strings.HasPrefix(path, prefix) { - return strings.TrimPrefix(path, prefix) - } - } - return path -} - -// transformSchemaContent removes folder prefixes from import statements in protobuf source -func transformSchemaContent(content string, prefixes []string) string { - modified := content - for _, prefix := range prefixes { - // Transform import statements like "workflows/v1/" to "v1/" - modified = strings.ReplaceAll(modified, `"`+prefix, `"`) - } - return modified -} - -// buildDependencyGraph builds a dependency graph from protobuf files -func buildDependencyGraph(protoMap map[string]string) (map[string][]string, error) { - dependencies := make(map[string][]string) - - framework.L.Debug().Msgf("Building dependency graph for %d proto files", len(protoMap)) - - // Initialize dependencies map - for path := range protoMap { - dependencies[path] = []string{} - } - - // Parse imports and build dependency graph - for path, content := range protoMap { - imports := extractImportStatements(content) - - for _, importPath := range imports { - if strings.HasPrefix(importPath, "google/protobuf/") { - // Skip Google protobuf imports as they're not in our protoMap - continue - } - - // Check if this import exists in our protoMap - if _, exists := protoMap[importPath]; exists { - // Check for self-reference - this indicates either an invalid proto file - // or a potential bug in our import/path handling - if importPath == path { - framework.L.Warn().Msgf("Self-reference detected: file %s imports itself (import: %s). This suggests either an invalid proto file or a path normalization issue. Skipping this dependency to avoid cycles.", path, importPath) - // Continue without adding the dependency to avoid cycles, but don't fail registration - // as this might be a recoverable issue or edge case - continue - } - - dependencies[path] = append(dependencies[path], importPath) - } else { - framework.L.Warn().Msgf("Import %s in %s not found in protoMap", importPath, path) - } - } - } - return dependencies, nil -} - -// topologicalSort performs topological sorting using Kahn's algorithm -func topologicalSort(dependencies map[string][]string) ([]string, error) { - // Calculate in-degrees (how many files each file depends on) - inDegree := make(map[string]int) - for file := range dependencies { - inDegree[file] = 0 - } - - // Count dependencies for each file - for file, deps := range dependencies { - inDegree[file] = len(deps) - } - - // Find files with no dependencies (in-degree = 0) - var queue []string - for file, degree := range inDegree { - if degree == 0 { - queue = append(queue, file) - } - } - - var result []string - for len(queue) > 0 { - file := queue[0] - queue = queue[1:] - result = append(result, file) - - // Reduce in-degree for files that depend on the current file - for dependent, deps := range dependencies { - for _, dep := range deps { - if dep == file { - inDegree[dependent]-- - if inDegree[dependent] == 0 { - queue = append(queue, dependent) - } - } - } - } - } - - // Check for cycles - if len(result) != len(dependencies) { - return nil, fmt.Errorf("circular dependency detected in protobuf files") - } - - return result, nil + return result.ID, nil } From 3d04e440ab23ee621c4dad407dc3810493cb56e1 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 11:39:26 +0200 Subject: [PATCH 2/6] run chip ingress test in CI, force ipv4 --- .../framework-dockercompose-tests.yml | 82 +++++++++++++++++++ .../dockercompose/chip_ingress_set/protos.go | 3 + .../myproject/smoke_chip_ingress_test.go | 3 - 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/framework-dockercompose-tests.yml diff --git a/.github/workflows/framework-dockercompose-tests.yml b/.github/workflows/framework-dockercompose-tests.yml new file mode 100644 index 000000000..fc7a8f38f --- /dev/null +++ b/.github/workflows/framework-dockercompose-tests.yml @@ -0,0 +1,82 @@ +name: Framework Docker Compose Tests +on: + push: + +jobs: + test: + defaults: + run: + working-directory: framework/examples/myproject + runs-on: ubuntu-latest + permissions: + id-token: write + contents: read + strategy: + fail-fast: false + matrix: + test: + - name: TestChipIngressSmoke + config: smoke_chip.toml + count: 1 + timeout: 10m + name: ${{ matrix.test.name }} + steps: + - name: Checkout repo + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + + - name: Configure AWS credentials using OIDC + uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 # v4.0.2 + with: + role-to-assume: ${{ secrets.AWS_CTF_READ_ACCESS_ROLE_ARN }} + aws-region: us-west-2 + + - name: Login to Amazon ECR + id: login-ecr-private + uses: aws-actions/amazon-ecr-login@062b18b96a7aff071d4dc91bc00c4c1a7945b076 # v2.0.1 + with: + registries: ${{ format('{0},{1}', secrets.AWS_ACCOUNT_ID_SDLC, secrets.AWS_ACCOUNT_ID_PROD) }} + env: + AWS_REGION: us-west-2 + + - name: Check for changes in Docker Components + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: changes + with: + filters: | + src: + - 'framework/components/dockercompose/**' + - '.github/workflows/framework-components-tests.yml' + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.24.0' + + - name: Cache Go modules + uses: actions/cache@v4 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: go-modules-${{ hashFiles('framework/examples/myproject/go.sum') }}-${{ runner.os }}-framework-golden-examples + restore-keys: | + go-modules-${{ runner.os }}-framework-golden-examples + go-modules-${{ runner.os }} + + - name: Install dependencies + run: go mod download + + - name: Run System Tests + if: steps.changes.outputs.src == 'true' + env: + CTF_CONFIGS: ${{ matrix.test.config }} + run: | + go test -timeout ${{ matrix.test.timeout }} -v -count ${{ matrix.test.count }} -run ${{ matrix.test.name }} + + - name: Upload Logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: container-logs-${{ matrix.test.name }} + path: framework/examples/myproject/logs + retention-days: 1 diff --git a/framework/components/dockercompose/chip_ingress_set/protos.go b/framework/components/dockercompose/chip_ingress_set/protos.go index d7b367b6c..7ad17acbb 100644 --- a/framework/components/dockercompose/chip_ingress_set/protos.go +++ b/framework/components/dockercompose/chip_ingress_set/protos.go @@ -510,6 +510,9 @@ func registerSingleProto( } url := fmt.Sprintf("%s/subjects/%s/versions", registryURL, subject) + // Force IPv4 to avoid Docker IPv6 port forwarding issues + url = strings.Replace(url, "localhost", "127.0.0.1", 1) + framework.L.Debug().Msgf("Registering schema to URL: %s", url) resp, respErr := http.Post(url, "application/vnd.schemaregistry.v1+json", bytes.NewReader(payload)) if respErr != nil { diff --git a/framework/examples/myproject/smoke_chip_ingress_test.go b/framework/examples/myproject/smoke_chip_ingress_test.go index cd6288599..c71883f94 100644 --- a/framework/examples/myproject/smoke_chip_ingress_test.go +++ b/framework/examples/myproject/smoke_chip_ingress_test.go @@ -2,7 +2,6 @@ package examples import ( "context" - "os" "testing" "time" @@ -17,8 +16,6 @@ type ChipConfig struct { // use config file: smoke_chip.toml func TestChipIngressSmoke(t *testing.T) { - t.Skip("skipping smoke test until we have a way to fetch Chip Ingress image") - os.Setenv("CTF_CONFIGS", "smoke_chip.toml") in, err := framework.Load[ChipConfig](t) require.NoError(t, err, "failed to load config") From 9a9e8218a96c23da5374bbd97728e8257f5f3d57 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 11:43:53 +0200 Subject: [PATCH 3/6] set env var --- .github/workflows/framework-dockercompose-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/framework-dockercompose-tests.yml b/.github/workflows/framework-dockercompose-tests.yml index fc7a8f38f..4167ac3ee 100644 --- a/.github/workflows/framework-dockercompose-tests.yml +++ b/.github/workflows/framework-dockercompose-tests.yml @@ -70,6 +70,7 @@ jobs: if: steps.changes.outputs.src == 'true' env: CTF_CONFIGS: ${{ matrix.test.config }} + CHIP_INGRESS_IMAGE: ${{ secrets.AWS_ACCOUNT_ID_PROD }}.dkr.ecr.us-west-2.amazonaws.com/atlas-chip-ingress:qa-latest run: | go test -timeout ${{ matrix.test.timeout }} -v -count ${{ matrix.test.count }} -run ${{ matrix.test.name }} From d32102b21d73efa943a1805b833d566f8fd4cb63 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 12:07:47 +0200 Subject: [PATCH 4/6] retry proto registration --- .../framework-dockercompose-tests.yml | 3 +- .../chip_ingress_set/chip_ingress.go | 1 + .../dockercompose/chip_ingress_set/protos.go | 40 ++++++++++++------- framework/components/dockercompose/go.mod | 1 + framework/components/dockercompose/go.sum | 2 + 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.github/workflows/framework-dockercompose-tests.yml b/.github/workflows/framework-dockercompose-tests.yml index 4167ac3ee..a9dbec656 100644 --- a/.github/workflows/framework-dockercompose-tests.yml +++ b/.github/workflows/framework-dockercompose-tests.yml @@ -18,7 +18,7 @@ jobs: - name: TestChipIngressSmoke config: smoke_chip.toml count: 1 - timeout: 10m + timeout: 3m name: ${{ matrix.test.name }} steps: - name: Checkout repo @@ -71,6 +71,7 @@ jobs: env: CTF_CONFIGS: ${{ matrix.test.config }} CHIP_INGRESS_IMAGE: ${{ secrets.AWS_ACCOUNT_ID_PROD }}.dkr.ecr.us-west-2.amazonaws.com/atlas-chip-ingress:qa-latest + CTF_LOG_LEVEL: debug run: | go test -timeout ${{ matrix.test.timeout }} -v -count ${{ matrix.test.count }} -run ${{ matrix.test.name }} diff --git a/framework/components/dockercompose/chip_ingress_set/chip_ingress.go b/framework/components/dockercompose/chip_ingress_set/chip_ingress.go index 29d2ba3c2..e54dcfcbb 100644 --- a/framework/components/dockercompose/chip_ingress_set/chip_ingress.go +++ b/framework/components/dockercompose/chip_ingress_set/chip_ingress.go @@ -128,6 +128,7 @@ func New(in *Input) (*Output, error) { wait.NewHostPortStrategy(DEFAULT_RED_PANDA_SCHEMA_REGISTRY_PORT).WithPollInterval(100*time.Millisecond), wait.NewHostPortStrategy(DEFAULT_RED_PANDA_KAFKA_PORT).WithPollInterval(100*time.Millisecond), wait.ForHTTP("/v1/status/ready").WithPort("9644"), // admin API port + wait.ForHTTP("/status/ready").WithPort(DEFAULT_RED_PANDA_SCHEMA_REGISTRY_PORT).WithPollInterval(100*time.Millisecond), ).WithDeadline(2*time.Minute), ).WaitForService(DEFAULT_RED_PANDA_CONSOLE_SERVICE_NAME, wait.ForAll( diff --git a/framework/components/dockercompose/chip_ingress_set/protos.go b/framework/components/dockercompose/chip_ingress_set/protos.go index 7ad17acbb..3723138b7 100644 --- a/framework/components/dockercompose/chip_ingress_set/protos.go +++ b/framework/components/dockercompose/chip_ingress_set/protos.go @@ -11,7 +11,9 @@ import ( "path/filepath" "regexp" "strings" + "time" + "github.com/avast/retry-go/v4" "github.com/google/go-github/v72/github" "github.com/pkg/errors" "github.com/smartcontractkit/chainlink-testing-framework/framework" @@ -510,23 +512,33 @@ func registerSingleProto( } url := fmt.Sprintf("%s/subjects/%s/versions", registryURL, subject) - // Force IPv4 to avoid Docker IPv6 port forwarding issues - url = strings.Replace(url, "localhost", "127.0.0.1", 1) - framework.L.Debug().Msgf("Registering schema to URL: %s", url) + maxAttempts := uint(10) - resp, respErr := http.Post(url, "application/vnd.schemaregistry.v1+json", bytes.NewReader(payload)) - if respErr != nil { - return 0, errors.Wrap(respErr, "failed to post to schema registry") - } - defer resp.Body.Close() + var resp *http.Response + retry.Do(func() error { + var respErr error + resp, respErr = http.Post(url, "application/vnd.schemaregistry.v1+json", bytes.NewReader(payload)) + if respErr != nil { + return errors.Wrap(respErr, "failed to post to schema registry") + } - if resp.StatusCode >= 300 { - data, dataErr := io.ReadAll(resp.Body) - if dataErr != nil { - return 0, errors.Wrap(dataErr, "failed to read response body") + if resp.StatusCode >= 300 { + data, dataErr := io.ReadAll(resp.Body) + if dataErr != nil { + return errors.Wrap(dataErr, "failed to read response body") + } + return fmt.Errorf("schema registry error (%d): %s", resp.StatusCode, data) } - return 0, fmt.Errorf("schema registry error (%d): %s", resp.StatusCode, data) - } + + return nil + }, retry.Attempts(maxAttempts), retry.Delay(100*time.Millisecond), retry.DelayType(retry.BackOffDelay), retry.OnRetry(func(n uint, err error) { + framework.L.Debug().Str("attempt/max", fmt.Sprintf("%d/%d", n, maxAttempts)).Msgf("Retrying to register schema %s: %v", subject, err) + }), retry.RetryIf(func(err error) bool { + // we don't want to retry all errors, because some of them are are expected (e.g. missing dependencies) + // and will be handled by higher-level code + return strings.Contains(err.Error(), "connection reset by peer") + })) + defer resp.Body.Close() var result struct { ID int `json:"id"` diff --git a/framework/components/dockercompose/go.mod b/framework/components/dockercompose/go.mod index 95c1cdc68..6e936ecb1 100644 --- a/framework/components/dockercompose/go.mod +++ b/framework/components/dockercompose/go.mod @@ -26,6 +26,7 @@ require ( github.com/Microsoft/go-winio v0.6.2 // indirect github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect + github.com/avast/retry-go/v4 v4.6.1 // indirect github.com/aws/aws-sdk-go-v2 v1.31.0 // indirect github.com/aws/aws-sdk-go-v2/config v1.27.39 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.17.37 // indirect diff --git a/framework/components/dockercompose/go.sum b/framework/components/dockercompose/go.sum index fdb418525..6b714d92d 100644 --- a/framework/components/dockercompose/go.sum +++ b/framework/components/dockercompose/go.sum @@ -40,6 +40,8 @@ github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= +github.com/avast/retry-go/v4 v4.6.1 h1:VkOLRubHdisGrHnTu89g08aQEWEgRU7LVEop3GbIcMk= +github.com/avast/retry-go/v4 v4.6.1/go.mod h1:V6oF8njAwxJ5gRo1Q7Cxab24xs5NCWZBeaHHBklR8mA= github.com/aws/aws-sdk-go-v2 v1.31.0 h1:3V05LbxTSItI5kUqNwhJrrrY1BAXxXt0sN0l72QmG5U= github.com/aws/aws-sdk-go-v2 v1.31.0/go.mod h1:ztolYtaEUtdpf9Wftr31CJfLVjOnD/CVRkKOOYgF8hA= github.com/aws/aws-sdk-go-v2/config v1.27.39 h1:FCylu78eTGzW1ynHcongXK9YHtoXD5AiiUqq3YfJYjU= From a4b7045c06f897e72d88cf73114b63d534e6fb89 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 12:24:21 +0200 Subject: [PATCH 5/6] go mod tidy --- framework/components/dockercompose/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/components/dockercompose/go.mod b/framework/components/dockercompose/go.mod index 6e936ecb1..0f727901c 100644 --- a/framework/components/dockercompose/go.mod +++ b/framework/components/dockercompose/go.mod @@ -5,6 +5,7 @@ go 1.24.2 replace github.com/smartcontractkit/chainlink-testing-framework/framework => ../../../framework require ( + github.com/avast/retry-go/v4 v4.6.1 github.com/confluentinc/confluent-kafka-go v1.9.2 github.com/docker/docker v28.0.4+incompatible github.com/google/go-github/v72 v72.0.0 @@ -26,7 +27,6 @@ require ( github.com/Microsoft/go-winio v0.6.2 // indirect github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect - github.com/avast/retry-go/v4 v4.6.1 // indirect github.com/aws/aws-sdk-go-v2 v1.31.0 // indirect github.com/aws/aws-sdk-go-v2/config v1.27.39 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.17.37 // indirect From 41ec08a96865123f3068049268dc5e1c5a716b64 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Thu, 28 Aug 2025 12:52:51 +0200 Subject: [PATCH 6/6] add changeset --- framework/components/dockercompose/.changeset/v0.1.12.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 framework/components/dockercompose/.changeset/v0.1.12.md diff --git a/framework/components/dockercompose/.changeset/v0.1.12.md b/framework/components/dockercompose/.changeset/v0.1.12.md new file mode 100644 index 000000000..3a326f754 --- /dev/null +++ b/framework/components/dockercompose/.changeset/v0.1.12.md @@ -0,0 +1,3 @@ +- Revert topological sorting of proto dependencies for the local-cre +- Revert support for exclude files to proto registration config +- Add retry on proto registration \ No newline at end of file