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

Move mutable configmaps out of deployment manifest #1983

Merged
merged 1 commit into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions build/yamls/antrea-aks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- apiregistration.k8s.io
resourceNames:
Expand Down Expand Up @@ -1312,22 +1318,6 @@ subjects:
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-ca
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-cluster-identity
namespace: kube-system
---
apiVersion: v1
data:
antrea-agent.conf: |
# FeatureGates is a map of feature names to bools that enable or disable experimental features.
Expand Down
22 changes: 6 additions & 16 deletions build/yamls/antrea-eks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- apiregistration.k8s.io
resourceNames:
Expand Down Expand Up @@ -1312,22 +1318,6 @@ subjects:
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-ca
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-cluster-identity
namespace: kube-system
---
apiVersion: v1
data:
antrea-agent.conf: |
# FeatureGates is a map of feature names to bools that enable or disable experimental features.
Expand Down
22 changes: 6 additions & 16 deletions build/yamls/antrea-gke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- apiregistration.k8s.io
resourceNames:
Expand Down Expand Up @@ -1312,22 +1318,6 @@ subjects:
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-ca
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-cluster-identity
namespace: kube-system
---
apiVersion: v1
data:
antrea-agent.conf: |
# FeatureGates is a map of feature names to bools that enable or disable experimental features.
Expand Down
22 changes: 6 additions & 16 deletions build/yamls/antrea-ipsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- apiregistration.k8s.io
resourceNames:
Expand Down Expand Up @@ -1312,22 +1318,6 @@ subjects:
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-ca
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-cluster-identity
namespace: kube-system
---
apiVersion: v1
data:
antrea-agent.conf: |
# FeatureGates is a map of feature names to bools that enable or disable experimental features.
Expand Down
22 changes: 6 additions & 16 deletions build/yamls/antrea.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- apiregistration.k8s.io
resourceNames:
Expand Down Expand Up @@ -1312,22 +1318,6 @@ subjects:
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-ca
namespace: kube-system
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: antrea
name: antrea-cluster-identity
namespace: kube-system
---
apiVersion: v1
data:
antrea-agent.conf: |
# FeatureGates is a map of feature names to bools that enable or disable experimental features.
Expand Down
6 changes: 6 additions & 0 deletions build/yamls/base/controller-rbac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
Comment on lines +86 to +91
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be missing something obvious but why don't we just add the "create" verb to the list above:

  - apiGroups:
      - ""
    resources:
      - configmaps
    resourceNames:
      - antrea-ca
      - antrea-cluster-identity
    verbs:
      - get
      - update
      - create

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered too, and thought maybe "create" does not work with resourceNames, but just checked and found it could. Then we should add create to the existing verb list.

Copy link
Contributor Author

@hty690 hty690 Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added "create" to this verb list and I met the error
error when creating 'kube-system/antrea-cluster-identity' ConfigMap: configmaps is forbidden: User "system:serviceaccount:kube-system:antrea-controller" cannot create resource "configmaps" in API group "" in the namespace "kube-system"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it works. I will recommit the changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying resourceNames for "create" verb doesn't work: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#referring-to-resources

Note: You cannot restrict create or deletecollection requests by resourceName. For create, this limitation is because the object name is not known at authorization time.

I did a test and it's consistent with the doc, how did you test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted and reapplied the yaml. And I found "antrea-ca" and "antrea-cluster-identity" was not deleted. and recreated. Maybe that's why I tested it with no error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then the current yaml makes sense.

- apiGroups:
- apiregistration.k8s.io
resources:
Expand Down
10 changes: 0 additions & 10 deletions build/yamls/base/controller.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,6 @@ spec:
selector:
component: antrea-controller
---
apiVersion: v1
kind: ConfigMap
metadata:
name: antrea-ca
---
apiVersion: v1
kind: ConfigMap
metadata:
name: antrea-cluster-identity
---
apiVersion: apiregistration.k8s.io/v1
kind: APIService
metadata:
Expand Down
14 changes: 6 additions & 8 deletions build/yamls/flow-aggregator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ rules:
verbs:
- get
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- apiGroups:
- ""
resourceNames:
Expand Down Expand Up @@ -119,14 +125,6 @@ subjects:
namespace: flow-aggregator
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app: flow-aggregator
name: flow-aggregator-ca
namespace: flow-aggregator
---
apiVersion: v1
data:
flow-aggregator.conf: |
# Provide the flow collector address as string with format <IP>:<port>[:<proto>], where proto is tcp or udp.
Expand Down
9 changes: 3 additions & 6 deletions build/yamls/flow-aggregator/base/flow-aggregator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ metadata:
name: flow-aggregator
---
apiVersion: v1
kind: ConfigMap
metadata:
name: flow-aggregator-ca
namespace: flow-aggregator
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: flow-aggregator
Expand All @@ -26,6 +20,9 @@ rules:
resources: ["configmaps"]
resourceNames: ["flow-aggregator-ca"]
verbs: ["get", "update"]
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["create"]
- apiGroups: [""]
resources: ["secrets"]
resourceNames: ["flow-aggregator-client-tls"]
Expand Down
26 changes: 23 additions & 3 deletions pkg/apiserver/certificate/cacert_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

v1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -245,17 +246,36 @@ func (c *CACertController) syncConfigMap(caCert []byte) error {
// Use the Antrea Pod Namespace for the CA cert ConfigMap.
caConfigMapNamespace := GetCAConfigMapNamespace()
caConfigMap, err := c.client.CoreV1().ConfigMaps(caConfigMapNamespace).Get(context.TODO(), CAConfigMapName, metav1.GetOptions{})
exists := true
if err != nil {
return fmt.Errorf("error getting ConfigMap %s: %v", CAConfigMapName, err)
if !errors.IsNotFound(err) {
return fmt.Errorf("error getting ConfigMap %s: %v", CAConfigMapName, err)
}
exists = false
caConfigMap = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: CAConfigMapName,
Namespace: caConfigMapNamespace,
Labels: map[string]string{
"app": "antrea",
},
},
}
}
if caConfigMap.Data != nil && caConfigMap.Data[CAConfigMapKey] == string(caCert) {
return nil
}
caConfigMap.Data = map[string]string{
CAConfigMapKey: string(caCert),
}
if _, err := c.client.CoreV1().ConfigMaps(caConfigMapNamespace).Update(context.TODO(), caConfigMap, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("error updating ConfigMap %s: %v", CAConfigMapName, err)
if exists {
if _, err := c.client.CoreV1().ConfigMaps(caConfigMapNamespace).Update(context.TODO(), caConfigMap, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("error updating ConfigMap %s: %v", CAConfigMapName, err)
}
} else {
if _, err := c.client.CoreV1().ConfigMaps(caConfigMapNamespace).Create(context.TODO(), caConfigMap, metav1.CreateOptions{}); err != nil {
return fmt.Errorf("error creating ConfigMap %s: %v", CAConfigMapName, err)
}
}
return nil
}
Expand Down
27 changes: 24 additions & 3 deletions pkg/clusteridentity/clusteridentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"time"

"github.com/google/uuid"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
clientset "k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -54,8 +56,21 @@ func NewClusterIdentityAllocator(

func (a *ClusterIdentityAllocator) updateConfigMapIfNeeded() error {
configMap, err := a.k8sClient.CoreV1().ConfigMaps(a.clusterIdentityConfigMapNamespace).Get(context.TODO(), a.clusterIdentityConfigMapName, metav1.GetOptions{})
exists := true
if err != nil {
return fmt.Errorf("error when getting '%s/%s' ConfigMap: %v", a.clusterIdentityConfigMapNamespace, a.clusterIdentityConfigMapName, err)
if !errors.IsNotFound(err) {
return fmt.Errorf("error when getting '%s/%s' ConfigMap: %v", a.clusterIdentityConfigMapNamespace, a.clusterIdentityConfigMapName, err)
}
exists = false
configMap = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: a.clusterIdentityConfigMapName,
Namespace: a.clusterIdentityConfigMapNamespace,
Labels: map[string]string{
"app": "antrea",
},
},
}
}

// returns a triplet consisting of the cluster UUID, a boolean indicating if the UUID needs
Expand Down Expand Up @@ -88,8 +103,14 @@ func (a *ClusterIdentityAllocator) updateConfigMapIfNeeded() error {
configMap.Data = map[string]string{
uuidConfigMapKey: clusterUUID.String(),
}
if _, err := a.k8sClient.CoreV1().ConfigMaps(a.clusterIdentityConfigMapNamespace).Update(context.TODO(), configMap, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("error when updating '%s/%s' ConfigMap with new cluster identity: %v", a.clusterIdentityConfigMapNamespace, a.clusterIdentityConfigMapName, err)
if exists {
if _, err := a.k8sClient.CoreV1().ConfigMaps(a.clusterIdentityConfigMapNamespace).Update(context.TODO(), configMap, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("error when updating '%s/%s' ConfigMap with new cluster identity: %v", a.clusterIdentityConfigMapNamespace, a.clusterIdentityConfigMapName, err)
}
} else {
if _, err := a.k8sClient.CoreV1().ConfigMaps(a.clusterIdentityConfigMapNamespace).Create(context.TODO(), configMap, metav1.CreateOptions{}); err != nil {
return fmt.Errorf("error when creating '%s/%s' ConfigMap with new cluster identity: %v", a.clusterIdentityConfigMapNamespace, a.clusterIdentityConfigMapName, err)
}
}
klog.Infof("New cluster UUID: %v", clusterUUID)
return nil
Expand Down
Loading