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

Fix RDS Redshift dynamic resources registration logic #11868

Merged
merged 1 commit into from
May 5, 2022

Conversation

smallinsky
Copy link
Contributor

@smallinsky smallinsky commented Apr 11, 2022

What

The RDS Redshift resource types registered dynamically by tctl create -f db-resource.yaml are evaluated agains DB Service Resource Matchers.

Only DB Cloud Originated resources should skip ResourceMatchers evaluation logic.

Issue: #11285

@smallinsky smallinsky force-pushed the smallinsky/fix_db_app_dynamic_resource_matcher branch 2 times, most recently from 170e2c8 to 7db474c Compare April 11, 2022 08:25
@smallinsky smallinsky requested review from r0mant and greedy52 April 12, 2022 09:02
@greedy52
Copy link
Contributor

In the original ticket, the sample config is

db_service:
  enabled: yes
  resources:
    - labels:
        env: prod
        account: someaccount

Is that

[]ResourceMatcher{                                                          
        {                                                                                     
            Labels: types.Labels{"env": []string{"prod"}},                                    
            Labels: types.Labels{"account": []string{"someaccount"}},                                    
        },                                                                                                       
    }  

Or

 []ResourceMatcher{                   
        {                        
            Labels: types.Labels{                                                                 
                "account": []string{"someaccount"},                                                         
                "env":     []string{"prod"},                                                                
            },                                                                                                 
        },                                                                                                     
    }  

I feel it's the later one?

@smallinsky smallinsky force-pushed the smallinsky/fix_db_app_dynamic_resource_matcher branch from 2ceb5bd to fd29b8b Compare April 22, 2022 11:57
@smallinsky smallinsky changed the title Fix dynamic app db resource matcher Fix RDS Redshift dynamic resources registration logic Apr 22, 2022
@smallinsky smallinsky requested a review from jakule April 22, 2022 12:03
@smallinsky smallinsky marked this pull request as ready for review April 22, 2022 12:05
@github-actions github-actions bot requested review from lxea and rosstimothy April 22, 2022 12:05
@github-actions github-actions bot added the database-access Database access related issues and PRs label Apr 22, 2022
lib/srv/db/watcher.go Outdated Show resolved Hide resolved
lib/srv/db/watcher.go Outdated Show resolved Hide resolved
lib/srv/db/watcher_test.go Outdated Show resolved Hide resolved
lib/srv/db/watcher.go Outdated Show resolved Hide resolved
lib/srv/db/watcher_test.go Outdated Show resolved Hide resolved
@smallinsky smallinsky force-pushed the smallinsky/fix_db_app_dynamic_resource_matcher branch from b430fab to 8a4cda2 Compare April 25, 2022 11:23
lib/srv/db/watcher.go Outdated Show resolved Hide resolved
@smallinsky smallinsky force-pushed the smallinsky/fix_db_app_dynamic_resource_matcher branch from 1668ab3 to 92ee0a1 Compare May 5, 2022 08:55
@smallinsky smallinsky merged commit 2a0cd20 into master May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

@smallinsky See the table below for backport results.

Branch Result
branch/v8 Create PR
branch/v9 Create PR

@webvictim webvictim mentioned this pull request Jun 8, 2022
@zmb3 zmb3 deleted the smallinsky/fix_db_app_dynamic_resource_matcher branch April 26, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-required database-access Database access related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants