Skip to content

Commit e938398

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #385 from sunzhaohua2/41827-1
OCPBUGS-41827: update injector to support secret as well
2 parents a4f2374 + 4501e46 commit e938398

6 files changed

Lines changed: 100 additions & 17 deletions

File tree

‎cmd/azure-config-credentials-injector/credentials_injector_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ func Test_mergeCloudConfig(t *testing.T) {
7272
{
7373
name: "AZURE_CLIENT_ID not set",
7474
args: []string{"--cloud-config-file-path", inputFile.Name(), "--output-file-path", outputFile.Name()},
75-
expectedErrMsg: "AZURE_CLIENT_ID env variable should be set up",
75+
expectedErrMsg: "azure_client_id should be set up",
7676
},
7777
{
7878
name: "AZURE_CLIENT_SECRET not set",
7979
args: []string{"--cloud-config-file-path", inputFile.Name(), "--output-file-path", outputFile.Name()},
8080
envVars: map[string]string{"AZURE_CLIENT_ID": "foo"},
81-
expectedErrMsg: "AZURE_CLIENT_SECRET env variable should be set up",
81+
expectedErrMsg: "azure_client_secret should be set up",
8282
},
8383
{
8484
name: "input file content is not a valid json",
@@ -161,19 +161,19 @@ func Test_mergeCloudConfig(t *testing.T) {
161161
name: "should fail, client secret is present while federated token file is present",
162162
args: []string{"--cloud-config-file-path", inputFile.Name(), "--output-file-path", outputFile.Name(), "--disable-identity-extension-auth", "--enable-azure-workload-identity=true"},
163163
envVars: map[string]string{"AZURE_TENANT_ID": "baz", "AZURE_CLIENT_ID": "foo", "AZURE_CLIENT_SECRET": "bar", "AZURE_FEDERATED_TOKEN_FILE": "baz"},
164-
expectedErrMsg: "AZURE_CLIENT_SECRET env variable is set while workload identity is enabled using AZURE_FEDERATED_TOKEN_FILE env variable, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com",
164+
expectedErrMsg: "azure_client_secret is set while workload identity is enabled using azure_federated_token_file, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com",
165165
},
166166
{
167167
name: "should fail, tenant id missing while federated token file is present",
168168
args: []string{"--cloud-config-file-path", inputFile.Name(), "--output-file-path", outputFile.Name(), "--disable-identity-extension-auth", "--enable-azure-workload-identity=true"},
169169
envVars: map[string]string{"AZURE_CLIENT_ID": "buzz", "AZURE_FEDERATED_TOKEN_FILE": "baz"},
170-
expectedErrMsg: "AZURE_TENANT_ID env variable should be set up while workload identity is enabled using AZURE_FEDERATED_TOKEN_FILE env variable, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com",
170+
expectedErrMsg: "azure_tenant_id should be set up while workload identity is enabled using azure_federated_token_file, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com",
171171
},
172172
{
173173
name: "should fail, workload identity can't be enabled because federated token missing, expect secret provided",
174174
args: []string{"--cloud-config-file-path", inputFile.Name(), "--output-file-path", outputFile.Name(), "--disable-identity-extension-auth", "--enable-azure-workload-identity=true"},
175175
envVars: map[string]string{"AZURE_TENANT_ID": "bar", "AZURE_CLIENT_ID": "buzz"},
176-
expectedErrMsg: "AZURE_CLIENT_SECRET env variable should be set up",
176+
expectedErrMsg: "azure_client_secret should be set up",
177177
},
178178
}
179179

‎cmd/azure-config-credentials-injector/main.go‎

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ var (
3737
outputFilePath string
3838
enableWorkloadIdentity string
3939
disableIdentityExtensionAuth bool
40+
credentialsPath string
4041
}
4142
)
4243

@@ -47,6 +48,7 @@ func init() {
4748
injectorCmd.PersistentFlags().StringVar(&injectorOpts.outputFilePath, "output-file-path", "/tmp/merged-cloud-config/cloud.conf", "Location of the generated cloud config file with injected credentials.")
4849
injectorCmd.PersistentFlags().BoolVar(&injectorOpts.disableIdentityExtensionAuth, "disable-identity-extension-auth", false, "Disable managed identity authentication, if it's set in cloudConfig.")
4950
injectorCmd.PersistentFlags().StringVar(&injectorOpts.enableWorkloadIdentity, "enable-azure-workload-identity", "false", "Enable workload identity authentication.")
51+
injectorCmd.PersistentFlags().StringVar(&injectorOpts.credentialsPath, "creds-path", "/etc/azure/credentials", "Path of the credential file.")
5052
}
5153

5254
func main() {
@@ -58,6 +60,7 @@ func main() {
5860
func mergeCloudConfig(_ *cobra.Command, args []string) error {
5961
var (
6062
azureClientId string
63+
azureClientIdFound bool
6164
tenantId string
6265
tenantIdFound bool
6366
federatedTokenFile string
@@ -71,30 +74,70 @@ func mergeCloudConfig(_ *cobra.Command, args []string) error {
7174
return err
7275
}
7376

74-
azureClientId, found := mustLookupEnvValue(clientIDEnvKey)
75-
if !found {
76-
return fmt.Errorf("%s env variable should be set up", clientIDEnvKey)
77+
// Read credentials from mounted Secret files or environment variables
78+
azureClientId, err = readSecretFile(fmt.Sprintf("%s/azure_client_id", injectorOpts.credentialsPath))
79+
if err != nil {
80+
if os.IsNotExist(err) {
81+
// Fallback to environment variable if secret file does not exist
82+
azureClientId, azureClientIdFound = mustLookupEnvValue(clientIDEnvKey)
83+
if !azureClientIdFound {
84+
return fmt.Errorf("azure_client_id should be set up")
85+
}
86+
} else {
87+
return fmt.Errorf("failed to read azure_client_id from secret: %w", err)
88+
}
7789
}
7890

79-
// Check for environment variables
80-
azureClientSecret, secretFound = mustLookupEnvValue(clientSecretEnvKey)
81-
federatedTokenFile, federatedTokenFileFound = mustLookupEnvValue(federatedTokenEnvKey)
82-
tenantId, tenantIdFound = mustLookupEnvValue(tenantIDEnvKey)
91+
azureClientSecret, err = readSecretFile(fmt.Sprintf("%s/azure_client_secret", injectorOpts.credentialsPath))
92+
if err != nil {
93+
if os.IsNotExist(err) {
94+
// Fallback to environment variable if secret file does not exist
95+
azureClientSecret, secretFound = mustLookupEnvValue(clientSecretEnvKey)
96+
} else {
97+
return fmt.Errorf("failed to read azure_client_secret from secret: %w", err)
98+
}
99+
} else {
100+
secretFound = true
101+
}
102+
103+
tenantId, err = readSecretFile(fmt.Sprintf("%s/azure_tenant_id", injectorOpts.credentialsPath))
104+
if err != nil {
105+
if os.IsNotExist(err) {
106+
// Fallback to environment variable if secret file does not exist
107+
tenantId, tenantIdFound = mustLookupEnvValue(tenantIDEnvKey)
108+
} else {
109+
return fmt.Errorf("failed to read azure_tenant_id from secret: %w", err)
110+
}
111+
} else {
112+
tenantIdFound = true
113+
}
114+
115+
federatedTokenFile, err = readSecretFile(fmt.Sprintf("%s/azure_federated_token_file", injectorOpts.credentialsPath))
116+
if err != nil {
117+
if os.IsNotExist(err) {
118+
// Fallback to environment variable if secret file does not exist
119+
federatedTokenFile, federatedTokenFileFound = mustLookupEnvValue(federatedTokenEnvKey)
120+
} else {
121+
return fmt.Errorf("failed to read azure_federated_token_file from secret: %w", err)
122+
}
123+
} else {
124+
federatedTokenFileFound = true
125+
}
83126

84127
// If federatedTokenFile found, workload identity should be used
85128
if federatedTokenFileFound {
86129
// azureClientSecret should not be set for workload identity auth, report error when secretFound
87130
if secretFound {
88-
return fmt.Errorf("%s env variable is set while workload identity is enabled using %s env variable, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com", clientSecretEnvKey, federatedTokenEnvKey)
131+
return fmt.Errorf("azure_client_secret is set while workload identity is enabled using azure_federated_token_file, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com")
89132
}
90133
// tenantId is required for workload identity auth, report error when !tenantIdFound
91134
if !tenantIdFound {
92-
return fmt.Errorf("%s env variable should be set up while workload identity is enabled using %s env variable, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com", tenantIDEnvKey, federatedTokenEnvKey)
135+
return fmt.Errorf("azure_tenant_id should be set up while workload identity is enabled using azure_federated_token_file, this should never happen.\nPlease consider reporting a bug: https://issues.redhat.com")
93136
}
94137
} else {
95138
// federatedTokenFile not found, secret will be required
96139
if !secretFound {
97-
return fmt.Errorf("%s env variable should be set up", clientSecretEnvKey)
140+
return fmt.Errorf("azure_client_secret should be set up")
98141
}
99142
}
100143

@@ -115,6 +158,15 @@ func mergeCloudConfig(_ *cobra.Command, args []string) error {
115158
return nil
116159
}
117160

161+
// Helper function to read a file and return its content as a string
162+
func readSecretFile(filePath string) (string, error) {
163+
data, err := os.ReadFile(filePath)
164+
if err != nil {
165+
return "", err
166+
}
167+
return string(data), nil
168+
}
169+
118170
func readCloudConfig(path string) (map[string]interface{}, error) {
119171
var data map[string]interface{}
120172

‎pkg/cloud/azure/assets/cloud-controller-manager-deployment.yaml‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ spec:
6969
--cloud-config-file-path=/etc/cloud-config/cloud.conf \
7070
--output-file-path=/etc/merged-cloud-config/cloud.conf \
7171
--disable-identity-extension-auth \
72-
--enable-azure-workload-identity=true
72+
--enable-azure-workload-identity=true \
73+
--creds-path=/etc/azure/credentials
7374
env:
7475
- name: AZURE_CLIENT_ID
7576
valueFrom:
@@ -101,6 +102,9 @@ spec:
101102
readOnly: true
102103
- name: merged-cloud-config
103104
mountPath: /etc/merged-cloud-config
105+
- name: cloud-sa-volume
106+
mountPath: /etc/azure/credentials
107+
readOnly: true
104108
containers:
105109
- name: cloud-controller-manager
106110
image: {{ .images.CloudControllerManager }}
@@ -156,6 +160,9 @@ spec:
156160
- name: merged-cloud-config
157161
mountPath: /etc/kubernetes-cloud-config
158162
readOnly: true
163+
- name: cloud-sa-volume
164+
readOnly: true
165+
mountPath: /etc/azure/credentials
159166
- name: trusted-ca
160167
mountPath: /etc/pki/ca-trust/extracted/pem
161168
readOnly: true
@@ -169,6 +176,9 @@ spec:
169176
items:
170177
- key: cloud.conf
171178
path: cloud.conf
179+
- name: cloud-sa-volume
180+
secret:
181+
secretName: azure-cloud-credentials
172182
- name: trusted-ca
173183
configMap:
174184
name: ccm-trusted-ca

‎pkg/cloud/azure/assets/cloud-node-manager-daemonset.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ spec:
6161
--cloud-config-file-path=/etc/cloud-config/cloud.conf \
6262
--output-file-path=/etc/merged-cloud-config/cloud.conf \
6363
--disable-identity-extension-auth \
64-
--enable-azure-workload-identity=true
64+
--enable-azure-workload-identity=true \
65+
--creds-path=/etc/azure/credentials
6566
env:
6667
- name: AZURE_CLIENT_ID
6768
valueFrom:
@@ -93,6 +94,9 @@ spec:
9394
readOnly: true
9495
- name: merged-cloud-config
9596
mountPath: /etc/merged-cloud-config
97+
- name: cloud-sa-volume
98+
mountPath: /etc/azure/credentials
99+
readOnly: true
96100
containers:
97101
- name: cloud-node-manager
98102
image: {{ .images.CloudNodeManager }}
@@ -135,6 +139,9 @@ spec:
135139
cpu: 50m
136140
memory: 50Mi
137141
volumes:
142+
- name: cloud-sa-volume
143+
secret:
144+
secretName: azure-cloud-credentials
138145
- name: trusted-ca
139146
configMap:
140147
name: ccm-trusted-ca

‎pkg/cloud/azurestack/assets/cloud-controller-manager-deployment.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ spec:
6161
args:
6262
- --cloud-config-file-path=/tmp/cloud-config/cloud.conf
6363
- --output-file-path=/tmp/merged-cloud-config/cloud.conf
64+
- --creds-path=/etc/azure/credentials
6465
env:
6566
- name: AZURE_CLIENT_ID
6667
valueFrom:
@@ -79,6 +80,9 @@ spec:
7980
readOnly: true
8081
- name: cloud-config
8182
mountPath: /tmp/merged-cloud-config
83+
- name: cloud-sa-volume
84+
mountPath: /etc/azure/credentials
85+
readOnly: true
8286
containers:
8387
- name: cloud-controller-manager
8488
image: {{ .images.CloudControllerManager }}
@@ -148,6 +152,9 @@ spec:
148152
hostPath:
149153
path: /etc/kubernetes
150154
type: Directory
155+
- name: cloud-sa-volume
156+
secret:
157+
secretName: azure-cloud-credentials
151158
- name: trusted-ca
152159
configMap:
153160
name: ccm-trusted-ca

‎pkg/cloud/azurestack/assets/cloud-node-manager-daemonset.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ spec:
5353
args:
5454
- --cloud-config-file-path=/tmp/cloud-config/cloud.conf
5555
- --output-file-path=/tmp/merged-cloud-config/cloud.conf
56+
- --creds-path=/etc/azure/credentials
5657
env:
5758
- name: AZURE_CLIENT_ID
5859
valueFrom:
@@ -71,6 +72,9 @@ spec:
7172
readOnly: true
7273
- name: cloud-config
7374
mountPath: /tmp/merged-cloud-config
75+
- name: cloud-sa-volume
76+
mountPath: /etc/azure/credentials
77+
readOnly: true
7478
containers:
7579
- name: cloud-node-manager
7680
image: {{ .images.CloudNodeManager }}
@@ -134,6 +138,9 @@ spec:
134138
path: cloud.conf
135139
- key: endpoints
136140
path: endpoints.conf
141+
- name: cloud-sa-volume
142+
secret:
143+
secretName: azure-cloud-credentials
137144
- name: trusted-ca
138145
configMap:
139146
name: ccm-trusted-ca

0 commit comments

Comments
 (0)