Add ignore aliaba specs which are not availble#2132
Add ignore aliaba specs which are not availble#2132cb-github-robot merged 3 commits intocloud-barista:mainfrom
Conversation
Signed-off-by: Seokho Son <shsongist@gmail.com>
Signed-off-by: seokho-son <shsongist@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements functionality to filter and ignore cloud specs that are not available during spec fetching operations. The changes add support for loading ignore patterns from a YAML configuration file and skip processing specs that match these patterns to improve performance and avoid unnecessary API calls.
Key changes:
- Add cloud spec ignore functionality with YAML configuration support
- Implement spec filtering based on provider and region-specific patterns
- Include ignore information in API responses and logging
- Improve HTTP response body logging with truncation for large responses
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/interface/rest/server/resource/image.go | Fix image key decoding logic to handle URL decode failures properly |
| src/interface/rest/docs/swagger.yaml | Add API schema for specs to ignore information |
| src/interface/rest/docs/swagger.json | Add JSON schema definitions for specs ignore data structure |
| src/interface/rest/docs/docs.go | Update generated documentation with new schema definitions |
| src/core/resource/spec.go | Implement core spec filtering logic and ignore pattern matching |
| src/core/model/spec.go | Add data models for ignore configuration and API responses |
| src/core/common/client/client.go | Improve HTTP response logging with better truncation handling |
| docker-compose.yaml | Enable external network access and metabase service |
| assets/cloudspec_ignore.yaml | Add comprehensive ignore patterns configuration for Alibaba Cloud |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| log.Debug().Msgf("[Get Image] (nsId: %s, Refined imageKey: %s)", nsId, imageKey) | ||
| imageKey = decodedImageKey |
There was a problem hiding this comment.
This assignment will use an empty string if URL decoding fails, potentially causing incorrect behavior. The original logic correctly preserved the original imageKey when decoding failed. This change could break image retrieval when the imageKey contains URL-encoded characters that fail to decode.
| defer func() { <-semaphore }() | ||
|
|
||
| common.RandomSleep(0, 10) | ||
| common.RandomSleep(0, 20) |
There was a problem hiding this comment.
The random sleep duration has been doubled from 10 to 20 seconds without explanation. This change could significantly impact performance by adding unnecessary delays. Consider documenting why this increase is needed or reverting to the original value.
| common.RandomSleep(0, 20) | |
| common.RandomSleep(0, 10) |
src/core/resource/spec.go
Outdated
| if strings.ToLower(providerName) == "alibaba" { | ||
| log.Debug(). | ||
| Str("provider", providerName). | ||
| Int("globalPatterns", len(cspPatterns.GlobalPatterns)). | ||
| Interface("globalPatterns", cspPatterns.GlobalPatterns). | ||
| Msg("Found CSP-specific patterns for alibaba") | ||
| } |
There was a problem hiding this comment.
This debug logging block is hardcoded specifically for 'alibaba' provider. This creates inconsistent debugging experience for other providers and should either be generalized to all providers or removed if it was temporary debugging code.
src/core/resource/spec.go
Outdated
| if strings.ToLower(providerName) == "alibaba" { | ||
| log.Debug(). | ||
| Str("spec", specName). | ||
| Str("pattern", pattern). | ||
| Str("provider", providerName). | ||
| Msg("Spec matched CSP global ignore pattern") | ||
| } |
There was a problem hiding this comment.
Another hardcoded debug logging block specific to 'alibaba'. These provider-specific debug logs should be generalized or removed to maintain consistent debugging across all cloud providers.
src/core/resource/spec.go
Outdated
| if strings.ToLower(providerName) == "alibaba" { | ||
| log.Debug(). | ||
| Str("spec", specName). | ||
| Str("pattern", pattern). | ||
| Str("provider", providerName). | ||
| Str("region", regionName). | ||
| Msg("Spec matched region-specific ignore pattern") | ||
| } |
There was a problem hiding this comment.
Third instance of alibaba-specific debug logging. These conditional debug logs create inconsistent behavior and should be either generalized to work for all providers or removed entirely.
| # CSP-specific ignore patterns | ||
| csps: | ||
| alibaba: | ||
| description: "Alibaba Cloud spec ignore patterns - Updated based on real availability data (89.6% deletion rate - Sequential cleanup)" |
There was a problem hiding this comment.
The title of the PR mentions 'aliaba' but this should be 'alibaba'. Please verify the PR title spelling is correct.
Signed-off-by: seokho-son <shsongist@gmail.com>
|
/approve |
No description provided.