Skip to content

Commit d182d6a

Browse files
tatlattatlat
authored
Merge pull request #632 from tatlat/pr-3-complete-interface-update
Use manifest to determine ECR
2 parents c26a334 + 8c37389 commit d182d6a

File tree

9 files changed

+286
-50
lines changed

9 files changed

+286
-50
lines changed

‎internal/aws/ecr/ecr.go‎

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88

99
"github.com/aws/aws-sdk-go-v2/aws"
1010
"github.com/aws/aws-sdk-go-v2/service/ecr"
11+
"go.uber.org/zap"
1112

13+
awsinternal "github.com/aws/eks-hybrid/internal/aws"
1214
"github.com/aws/eks-hybrid/internal/aws/imds"
1315
"github.com/aws/eks-hybrid/internal/system"
1416
)
@@ -30,21 +32,21 @@ func (r *ECRRegistry) GetSandboxImage() string {
3032
return r.GetImageReference("eks/pause", "3.5")
3133
}
3234

33-
func GetEKSRegistry(region string) (ECRRegistry, error) {
35+
func GetEKSRegistry(region string, regionConfig *awsinternal.RegionData) (ECRRegistry, error) {
3436
servicesDomain, err := imds.GetProperty(imds.ServicesDomain)
3537
if err != nil {
3638
return "", err
3739
}
3840

39-
return getEksRegistryWithServiceDomain(region, servicesDomain)
41+
return getEksRegistryWithServiceDomainAndRegionConfig(region, servicesDomain, regionConfig)
4042
}
4143

42-
func GetEKSHybridRegistry(region string) (ECRRegistry, error) {
43-
return getEksRegistryWithServiceDomain(region, hybridServicesDomain)
44+
func GetEKSHybridRegistry(region string, regionConfig *awsinternal.RegionData) (ECRRegistry, error) {
45+
return getEksRegistryWithServiceDomainAndRegionConfig(region, hybridServicesDomain, regionConfig)
4446
}
4547

46-
func getEksRegistryWithServiceDomain(region, servicesDomain string) (ECRRegistry, error) {
47-
account, region := getEKSRegistryCoordinates(region)
48+
func getEksRegistryWithServiceDomainAndRegionConfig(region, servicesDomain string, regionConfig *awsinternal.RegionData) (ECRRegistry, error) {
49+
account, region := getEKSRegistryCoordinatesWithRegionConfig(region, regionConfig)
4850
fipsInstalled, fipsEnabled, err := system.GetFipsInfo()
4951
if err != nil {
5052
return "", err
@@ -77,43 +79,9 @@ func getRegistry(accountID, ecrSubdomain, region, servicesDomain string) string
7779
const nonOptInRegionAccount = "602401143452"
7880

7981
var accountsByRegion = map[string]string{
80-
"ap-northeast-1": nonOptInRegionAccount,
81-
"ap-northeast-2": nonOptInRegionAccount,
82-
"ap-northeast-3": nonOptInRegionAccount,
83-
"ap-south-1": nonOptInRegionAccount,
84-
"ap-southeast-1": nonOptInRegionAccount,
85-
"ap-southeast-2": nonOptInRegionAccount,
86-
"ca-central-1": nonOptInRegionAccount,
87-
"eu-central-1": nonOptInRegionAccount,
88-
"eu-north-1": nonOptInRegionAccount,
89-
"eu-west-1": nonOptInRegionAccount,
90-
"eu-west-2": nonOptInRegionAccount,
91-
"eu-west-3": nonOptInRegionAccount,
92-
"sa-east-1": nonOptInRegionAccount,
93-
"us-east-1": nonOptInRegionAccount,
94-
"us-east-2": nonOptInRegionAccount,
95-
"us-west-1": nonOptInRegionAccount,
96-
"us-west-2": nonOptInRegionAccount,
97-
98-
"af-south-1": "877085696533",
99-
"ap-east-1": "800184023465",
100-
"ap-east-2": "533267051163",
101-
"ap-south-2": "900889452093",
102-
"ap-southeast-3": "296578399912",
103-
"ap-southeast-4": "491585149902",
104-
"ap-southeast-5": "151610086707",
105-
"ap-southeast-7": "121268973566",
106-
"ca-west-1": "761377655185",
10782
"cn-north-1": "918309763551",
10883
"cn-northwest-1": "961992271922",
109-
"eu-central-2": "900612956339",
11084
"eu-isoe-west-1": "249663109785",
111-
"eu-south-1": "590381155156",
112-
"eu-south-2": "455263428931",
113-
"il-central-1": "066635153087",
114-
"me-central-1": "759879836304",
115-
"me-south-1": "558608220178",
116-
"mx-central-1": "730335286997",
11785
"us-gov-east-1": "151742754352",
11886
"us-gov-west-1": "013241004608",
11987
"us-iso-east-1": "725322719131",
@@ -128,6 +96,8 @@ func getEKSRegistryCoordinates(region string) (string, string) {
12896
if ok {
12997
return inRegionRegistry, region
13098
}
99+
100+
// Fallback to existing region prefix-based logic
131101
if strings.HasPrefix(region, "us-gov-") {
132102
return "013241004608", "us-gov-west-1"
133103
} else if strings.HasPrefix(region, "cn-") {
@@ -139,5 +109,31 @@ func getEKSRegistryCoordinates(region string) (string, string) {
139109
} else if strings.HasPrefix(region, "us-isof-") {
140110
return "676585237158", "us-isof-south-1"
141111
}
142-
return "602401143452", "us-west-2"
112+
return nonOptInRegionAccount, "us-west-2"
113+
}
114+
115+
// getEKSRegistryCoordinatesWithRegionConfig returns an AWS region and account ID for the default EKS ECR container image registry
116+
// using region configuration from manifest when available
117+
func getEKSRegistryCoordinatesWithRegionConfig(region string, regionConfig *awsinternal.RegionData) (string, string) {
118+
if regionConfig != nil && regionConfig.EcrAccountID != "" {
119+
zap.L().Info("Using ECR account from manifest",
120+
zap.String("region", region),
121+
zap.String("ecrAccount", regionConfig.EcrAccountID))
122+
return regionConfig.EcrAccountID, region
123+
}
124+
125+
// Fall back to existing logic
126+
account, fallbackRegion := getEKSRegistryCoordinates(region)
127+
if account == nonOptInRegionAccount {
128+
zap.L().Warn("Region not found in manifest. Attempting to use default ECR account...")
129+
zap.L().Info("Using default non-opt-in region ECR account",
130+
zap.String("requestedRegion", region),
131+
zap.String("fallbackRegion", fallbackRegion),
132+
zap.String("ecrAccount", account))
133+
} else {
134+
zap.L().Info("Using region-specific ECR account",
135+
zap.String("region", region),
136+
zap.String("ecrAccount", account))
137+
}
138+
return account, fallbackRegion
143139
}

‎internal/aws/ecr/ecr_test.go‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package ecr
2+
3+
import (
4+
"testing"
5+
6+
awsinternal "github.com/aws/eks-hybrid/internal/aws"
7+
)
8+
9+
func TestGetEKSRegistryCoordinates(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
region string
13+
regionConfig *awsinternal.RegionData
14+
expectAcct string
15+
expectRegion string
16+
}{
17+
{
18+
name: "manifest data available - takes precedence",
19+
region: "us-east-1",
20+
regionConfig: &awsinternal.RegionData{
21+
EcrAccountID: "123456789012",
22+
},
23+
expectAcct: "123456789012",
24+
expectRegion: "us-east-1",
25+
},
26+
{
27+
name: "manifest data available for non-commercial region - takes precedence over hardcoded",
28+
region: "cn-north-1",
29+
regionConfig: &awsinternal.RegionData{
30+
EcrAccountID: "123456789012",
31+
},
32+
expectAcct: "123456789012",
33+
expectRegion: "cn-north-1",
34+
},
35+
{
36+
name: "non-commercial region - cn-north-1 from hardcoded map",
37+
region: "cn-north-1",
38+
regionConfig: nil,
39+
expectAcct: "918309763551",
40+
expectRegion: "cn-north-1",
41+
},
42+
{
43+
name: "non-commercial region - cn-northwest-1 from hardcoded map",
44+
region: "cn-northwest-1",
45+
regionConfig: nil,
46+
expectAcct: "961992271922",
47+
expectRegion: "cn-northwest-1",
48+
},
49+
{
50+
name: "non-commercial region - us-gov-east-1 from hardcoded map",
51+
region: "us-gov-east-1",
52+
regionConfig: nil,
53+
expectAcct: "151742754352",
54+
expectRegion: "us-gov-east-1",
55+
},
56+
{
57+
name: "non-commercial region - us-gov-west-1 from hardcoded map",
58+
region: "us-gov-west-1",
59+
regionConfig: nil,
60+
expectAcct: "013241004608",
61+
expectRegion: "us-gov-west-1",
62+
},
63+
{
64+
name: "non-commercial region - us-iso-east-1 from hardcoded map",
65+
region: "us-iso-east-1",
66+
regionConfig: nil,
67+
expectAcct: "725322719131",
68+
expectRegion: "us-iso-east-1",
69+
},
70+
{
71+
name: "non-commercial region - us-iso-west-1 from hardcoded map",
72+
region: "us-iso-west-1",
73+
regionConfig: nil,
74+
expectAcct: "608367168043",
75+
expectRegion: "us-iso-west-1",
76+
},
77+
{
78+
name: "non-commercial region - us-isob-east-1 from hardcoded map",
79+
region: "us-isob-east-1",
80+
regionConfig: nil,
81+
expectAcct: "187977181151",
82+
expectRegion: "us-isob-east-1",
83+
},
84+
{
85+
name: "non-commercial region - us-isof-south-1 from hardcoded map",
86+
region: "us-isof-south-1",
87+
regionConfig: nil,
88+
expectAcct: "676585237158",
89+
expectRegion: "us-isof-south-1",
90+
},
91+
{
92+
name: "non-commercial region - eu-isoe-west-1 from hardcoded map",
93+
region: "eu-isoe-west-1",
94+
regionConfig: nil,
95+
expectAcct: "249663109785",
96+
expectRegion: "eu-isoe-west-1",
97+
},
98+
{
99+
name: "prefix fallback - unknown us-gov region",
100+
region: "us-gov-unknown-1",
101+
regionConfig: nil,
102+
expectAcct: "013241004608",
103+
expectRegion: "us-gov-west-1",
104+
},
105+
{
106+
name: "prefix fallback - unknown cn region",
107+
region: "cn-unknown-1",
108+
regionConfig: nil,
109+
expectAcct: "961992271922",
110+
expectRegion: "cn-northwest-1",
111+
},
112+
{
113+
name: "prefix fallback - unknown us-iso region",
114+
region: "us-iso-unknown-1",
115+
regionConfig: nil,
116+
expectAcct: "725322719131",
117+
expectRegion: "us-iso-east-1",
118+
},
119+
{
120+
name: "prefix fallback - unknown us-isob region",
121+
region: "us-isob-unknown-1",
122+
regionConfig: nil,
123+
expectAcct: "187977181151",
124+
expectRegion: "us-isob-east-1",
125+
},
126+
{
127+
name: "prefix fallback - unknown us-isof region",
128+
region: "us-isof-unknown-1",
129+
regionConfig: nil,
130+
expectAcct: "676585237158",
131+
expectRegion: "us-isof-south-1",
132+
},
133+
{
134+
name: "default fallback - commercial region not in manifest",
135+
region: "us-east-1",
136+
regionConfig: nil,
137+
expectAcct: "602401143452",
138+
expectRegion: "us-west-2",
139+
},
140+
{
141+
name: "default fallback - unknown region",
142+
region: "unknown-region",
143+
regionConfig: nil,
144+
expectAcct: "602401143452",
145+
expectRegion: "us-west-2",
146+
},
147+
{
148+
name: "empty ECR account ID in manifest - falls back to hardcoded for non-commercial",
149+
region: "cn-north-1",
150+
regionConfig: &awsinternal.RegionData{
151+
EcrAccountID: "",
152+
},
153+
expectAcct: "918309763551",
154+
expectRegion: "cn-north-1",
155+
},
156+
{
157+
name: "empty ECR account ID in manifest - falls back to default for commercial",
158+
region: "us-east-1",
159+
regionConfig: &awsinternal.RegionData{
160+
EcrAccountID: "",
161+
},
162+
expectAcct: "602401143452",
163+
expectRegion: "us-west-2",
164+
},
165+
}
166+
167+
for _, tt := range tests {
168+
t.Run(tt.name, func(t *testing.T) {
169+
account, region := getEKSRegistryCoordinatesWithRegionConfig(tt.region, tt.regionConfig)
170+
if account != tt.expectAcct {
171+
t.Errorf("Expected account %s, got %s", tt.expectAcct, account)
172+
}
173+
if region != tt.expectRegion {
174+
t.Errorf("Expected region %s, got %s", tt.expectRegion, region)
175+
}
176+
})
177+
}
178+
}

‎internal/aws/source.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,20 @@ func getLatestDateEksPatchRelease(patchReleases []EksPatchRelease) (EksPatchRele
138138
return latestRelease, nil
139139
}
140140

141+
func GetRegionConfig(ctx context.Context, region string) (*RegionData, error) {
142+
manifest, err := getReleaseManifest(ctx)
143+
if err != nil {
144+
return nil, err
145+
}
146+
147+
regionCfg, ok := manifest.RegionConfig[region]
148+
if !ok {
149+
return nil, fmt.Errorf("region %s not found in manifest", region)
150+
}
151+
152+
return &regionCfg, nil
153+
}
154+
141155
// GetKubelet satisfies kubelet.Source.
142156
func (as Source) GetKubelet(ctx context.Context) (artifact.Source, error) {
143157
return as.getEksSource(ctx, "kubelet")

‎internal/configenricher/interface.go‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,26 @@
11
package configenricher
22

3-
import "context"
3+
import (
4+
"context"
5+
6+
"github.com/aws/eks-hybrid/internal/aws"
7+
)
8+
9+
// ConfigEnricherConfig holds the configuration options
10+
type ConfigEnricherConfig struct {
11+
RegionConfig *aws.RegionData
12+
}
13+
14+
// ConfigEnricherOption is a function that modifies ConfigEnricherConfig
15+
type ConfigEnricherOption func(*ConfigEnricherConfig)
16+
17+
// WithRegionConfig creates a ConfigEnricherOption that sets the region config
18+
func WithRegionConfig(regionConfig *aws.RegionData) ConfigEnricherOption {
19+
return func(config *ConfigEnricherConfig) {
20+
config.RegionConfig = regionConfig
21+
}
22+
}
423

524
type ConfigEnricher interface {
6-
Enrich(ctx context.Context) error
25+
Enrich(ctx context.Context, opts ...ConfigEnricherOption) error
726
}

‎internal/flows/init.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"go.uber.org/zap"
77
"k8s.io/utils/strings/slices"
88

9+
"github.com/aws/eks-hybrid/internal/aws"
10+
"github.com/aws/eks-hybrid/internal/configenricher"
911
"github.com/aws/eks-hybrid/internal/nodeprovider"
1012
)
1113

@@ -33,7 +35,14 @@ func (i *Initer) Run(ctx context.Context) error {
3335
return err
3436
}
3537

36-
if err := i.NodeProvider.Enrich(ctx); err != nil {
38+
// Get region config from manifest for ECR registry lookup
39+
region := i.NodeProvider.GetNodeConfig().Spec.Cluster.Region
40+
regionConfig, err := aws.GetRegionConfig(ctx, region)
41+
if err != nil {
42+
i.Logger.Warn("Failed to get region config from manifest", zap.Error(err))
43+
}
44+
45+
if err := i.NodeProvider.Enrich(ctx, configenricher.WithRegionConfig(regionConfig)); err != nil {
3746
return err
3847
}
3948

‎internal/flows/upgrade.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/aws/eks-hybrid/internal/aws"
1111
"github.com/aws/eks-hybrid/internal/cni"
12+
"github.com/aws/eks-hybrid/internal/configenricher"
1213
"github.com/aws/eks-hybrid/internal/containerd"
1314
"github.com/aws/eks-hybrid/internal/creds"
1415
"github.com/aws/eks-hybrid/internal/daemon"
@@ -51,7 +52,8 @@ func (u *Upgrader) Run(ctx context.Context) error {
5152
if err := u.NodeProvider.ConfigureAws(ctx); err != nil {
5253
return err
5354
}
54-
if err := u.NodeProvider.Enrich(ctx); err != nil {
55+
56+
if err := u.NodeProvider.Enrich(ctx, configenricher.WithRegionConfig(&u.AwsSource.RegionInfo)); err != nil {
5557
return err
5658
}
5759
if err := initDaemons(ctx, u.NodeProvider, u.SkipPhases, u.Logger); err != nil {

0 commit comments

Comments
 (0)