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.

Sign up for GitHub

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

add ASO install #3450

Merged
merged 1 commit into from
May 8, 2023
Merged

add ASO install #3450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,8 @@ docker-pull-prerequisites: ## Pull prerequisites for building controller-manager
.PHONY: docker-build
docker-build: docker-pull-prerequisites ## Build the docker image for controller-manager.
DOCKER_BUILDKIT=1 docker build --build-arg goproxy=$(GOPROXY) --build-arg ARCH=$(ARCH) --build-arg ldflags="$(LDFLAGS)" . -t $(CONTROLLER_IMG)-$(ARCH):$(TAG)
$(MAKE) set-manifest-image MANIFEST_IMG=$(CONTROLLER_IMG)-$(ARCH) MANIFEST_TAG=$(TAG) TARGET_RESOURCE="./config/default/manager_image_patch.yaml"
$(MAKE) set-manifest-pull-policy TARGET_RESOURCE="./config/default/manager_pull_policy.yaml"
$(MAKE) set-manifest-image MANIFEST_IMG=$(CONTROLLER_IMG)-$(ARCH) MANIFEST_TAG=$(TAG) TARGET_RESOURCE="./config/capz/manager_image_patch.yaml"
$(MAKE) set-manifest-pull-policy TARGET_RESOURCE="./config/capz/manager_pull_policy.yaml"

.PHONY: docker-push
docker-push: ## Push the docker image
Expand Down Expand Up @@ -412,12 +412,12 @@ docker-push-manifest: ## Push the fat manifest docker image.
.PHONY: set-manifest-image
set-manifest-image: ## Update kustomize image patch file for default resource.
$(info Updating kustomize image patch file for default resource)
sed -i'' -e 's@image: .*@image: '"${MANIFEST_IMG}:$(MANIFEST_TAG)"'@' ./config/default/manager_image_patch.yaml
sed -i'' -e 's@image: .*@image: '"${MANIFEST_IMG}:$(MANIFEST_TAG)"'@' ./config/capz/manager_image_patch.yaml

.PHONY: set-manifest-pull-policy
set-manifest-pull-policy: ## Update kustomize pull policy file for default resource.
$(info Updating kustomize pull policy file for default resource)
sed -i'' -e 's@imagePullPolicy: .*@imagePullPolicy: '"$(PULL_POLICY)"'@' ./config/default/manager_pull_policy.yaml
sed -i'' -e 's@imagePullPolicy: .*@imagePullPolicy: '"$(PULL_POLICY)"'@' ./config/capz/manager_pull_policy.yaml

## --------------------------------------
## Generate
Expand Down Expand Up @@ -686,8 +686,8 @@ test-e2e-skip-push: ## Run "docker-build" rule then run e2e tests.

.PHONY: test-e2e-skip-build-and-push
test-e2e-skip-build-and-push:
$(MAKE) set-manifest-image MANIFEST_IMG=$(CONTROLLER_IMG)-$(ARCH) MANIFEST_TAG=$(TAG) TARGET_RESOURCE="./config/default/manager_image_patch.yaml"
$(MAKE) set-manifest-pull-policy TARGET_RESOURCE="./config/default/manager_pull_policy.yaml" PULL_POLICY=IfNotPresent
$(MAKE) set-manifest-image MANIFEST_IMG=$(CONTROLLER_IMG)-$(ARCH) MANIFEST_TAG=$(TAG) TARGET_RESOURCE="./config/capz/manager_image_patch.yaml"
$(MAKE) set-manifest-pull-policy TARGET_RESOURCE="./config/capz/manager_pull_policy.yaml" PULL_POLICY=IfNotPresent
MANAGER_IMAGE=$(CONTROLLER_IMG)-$(ARCH):$(TAG) \
$(MAKE) test-e2e-run

Expand Down
10 changes: 10 additions & 0 deletions config/aso/credentials.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
apiVersion: v1
kind: Secret
metadata:
name: aso-controller-settings
type: Opaque
data:
AZURE_SUBSCRIPTION_ID: ${AZURE_SUBSCRIPTION_ID_B64:=""}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this temporary until we can use workload identity?

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 was thinking auth for ASO would essentially mirror the AzureClusterIdentity for the Cluster, which could also be Service Principal. Or would it make sense to always configure ASO to use workload identity? I still don't have a clear idea of exactly how workload identity works so I couldn't quite figure out how to get that set up based on the docs: https://azure.github.io/azure-service-operator/guide/authentication/#azure-workload-identity

I was also thinking having a default set of credentials here would make it easy for users to get started using ASO directly even if CAPZ can work around relying on the credentials here.

AZURE_TENANT_ID: ${AZURE_TENANT_ID_B64:=""}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, is there a reason we want to add the :="" part to the variable references?

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 admit this was a shameless copy-paste from here and didn't really consider whether it's necessary or not:

cluster-api-provider-azure/config/default/credentials.yaml

Lines 7 to 8 in fec6314

data:
subscription-id: ${AZURE_SUBSCRIPTION_ID_B64:=""}

It looks like ASO will get stuck in a crash loop either way when the vars aren't defined, but with :="" it doesn't crash until an ASO resource is created, whereas without :="" it'll crash as soon as it starts up even when no ASO resources exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the CAPZ manager credentials, the idea is that you can optionally define global credentials but it's also possible to define per Cluster credentials via AzureClusterIdentity, so we don't want the credentials to be required when installing CAPZ as they aren't actually required until you create your first cluster (without the defaulting to "", which is what :="" does, clusterctl init would fail when the environment variables are not set).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I think with :="" is what we want then. Eventually, a set of ASO credentials will be generated for each individual cluster from an AzureClusterIdentity if one exists, so those users can still choose not to define the global credentials and ASO will work fine with CAPZ until the user tries to create an ASO resource themselves without specifying credentials and relying on the defaults.

AZURE_CLIENT_ID: ${AZURE_CLIENT_ID_B64:=""}
AZURE_CLIENT_SECRET: ${AZURE_CLIENT_SECRET_B64:=""}
65 changes: 65 additions & 0 deletions config/aso/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
apiVersion: kustomize.config.k8s.io/v1alpha1
kind: Component
namespace: capz-system
resources:
- https://github.com/Azure/azure-service-operator/releases/download/v2.0.0/azureserviceoperator_v2.0.0.yaml
- https://github.com/Azure/azure-service-operator/releases/download/v2.0.0/azureserviceoperator_customresourcedefinitions_v2.0.0.yaml
- credentials.yaml

patches:
- patch: |- # default kustomization includes a namespace already
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: do we want a line break at the end? Not sure if it's better to use |- or | in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like that doesn't affect the output at all.

$patch: delete
apiVersion: v1
kind: Namespace
metadata:
name: capz-system
- patch: |- # CAPZ will manage ASO's CRDs
- op: test
path: /spec/template/spec/containers/0/args/4
value: --crd-pattern=*
- op: remove
path: /spec/template/spec/containers/0/args/4
target:
group: apps
version: v1
kind: Deployment
name: azureserviceoperator-controller-manager
- patch: |- # remove permissions to manage CRDs
$patch: delete
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: azureserviceoperator-crd-manager-role
- patch: |-
$patch: delete
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: azureserviceoperator-crd-manager-rolebinding

replacements:
- source:
kind: Certificate
group: cert-manager.io
version: v1
name: azureserviceoperator-serving-cert
fieldPath: metadata.namespace
targets:
- select:
version: v1
fieldPaths:
- metadata.annotations.cert-manager\.io/inject-ca-from
options:
delimiter: /
index: 0
- select:
group: cert-manager.io
version: v1
kind: Certificate
name: azureserviceoperator-serving-cert
fieldPaths:
- spec.dnsNames.0
- spec.dnsNames.1
options:
delimiter: .
index: 1
0 .../default/aad-pod-identity-deployment.yaml → config/capz/aad-pod-identity-deployment.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/credentials.yaml → config/capz/credentials.yaml
View file
Open in desktop
File renamed without changes.
57 changes: 57 additions & 0 deletions config/capz/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
namespace: capz-system

namePrefix: capz-

# Labels to add to all resources and selectors.
commonLabels:
cluster.x-k8s.io/provider: "infrastructure-azure"

resources:
- namespace.yaml
- credentials.yaml
- aad-pod-identity-deployment.yaml

bases:
- ../crd
- ../rbac
- ../manager
- ../webhook
- ../certmanager

patchesStrategicMerge:
- manager_image_patch.yaml
- manager_pull_policy.yaml
- manager_credentials_patch.yaml
- manager_webhook_patch.yaml
- webhookcainjection_patch.yaml

vars:
- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
fieldref:
fieldpath: metadata.namespace
- name: CERTIFICATE_NAME
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
- name: SERVICE_NAMESPACE # namespace of the service
objref:
kind: Service
version: v1
name: webhook-service
fieldref:
fieldpath: metadata.namespace
- name: SERVICE_NAME
objref:
kind: Service
version: v1
name: webhook-service

configurations:
- kustomizeconfig.yaml
0 config/default/kustomizeconfig.yaml → config/capz/kustomizeconfig.yaml
View file
Open in desktop
File renamed without changes.
0 ...ig/default/manager_credentials_patch.yaml → config/capz/manager_credentials_patch.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/manager_image_patch.yaml → config/capz/manager_image_patch.yaml
View file
Open in desktop
File renamed without changes.
0 ...ult/manager_prometheus_metrics_patch.yaml → ...apz/manager_prometheus_metrics_patch.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/manager_pull_policy.yaml → config/capz/manager_pull_policy.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/manager_webhook_patch.yaml → config/capz/manager_webhook_patch.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/namespace.yaml → config/capz/namespace.yaml
View file
Open in desktop
File renamed without changes.
0 config/default/webhookcainjection_patch.yaml → config/capz/webhookcainjection_patch.yaml
View file
Open in desktop
File renamed without changes.
57 changes: 1 addition & 56 deletions config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,2 @@
namespace: capz-system

namePrefix: capz-

# Labels to add to all resources and selectors.
commonLabels:
cluster.x-k8s.io/provider: "infrastructure-azure"

resources:
- namespace.yaml
- credentials.yaml
- aad-pod-identity-deployment.yaml

bases:
- ../crd
- ../rbac
- ../manager
- ../webhook
- ../certmanager

patchesStrategicMerge:
- manager_image_patch.yaml
- manager_pull_policy.yaml
- manager_credentials_patch.yaml
- manager_webhook_patch.yaml
- webhookcainjection_patch.yaml

vars:
- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
fieldref:
fieldpath: metadata.namespace
- name: CERTIFICATE_NAME
objref:
kind: Certificate
group: cert-manager.io
version: v1
name: serving-cert # this name should match the one in certificate.yaml
- name: SERVICE_NAMESPACE # namespace of the service
objref:
kind: Service
version: v1
name: webhook-service
fieldref:
fieldpath: metadata.namespace
- name: SERVICE_NAME
objref:
kind: Service
version: v1
name: webhook-service

configurations:
- kustomizeconfig.yaml
- ../capz