Skip to content

Fix/cleanup pre commit findings - #100

Open
rkage wants to merge 3 commits into
mainfrom
fix/cleanup-pre-commit-findings
Open

Fix/cleanup pre commit findings#100
rkage wants to merge 3 commits into
mainfrom
fix/cleanup-pre-commit-findings

Conversation

@rkage

@rkage rkage commented Feb 1, 2021

Copy link
Copy Markdown
Member

Description

Cleans up some pre-commit findings.. @anthr76 - have a look since this removes private keys.

rkage added 3 commits January 31, 2021 16:25
Signed-off-by: Nick M <4718+rkage@users.noreply.github.com>
Signed-off-by: Nick M <4718+rkage@users.noreply.github.com>
Signed-off-by: Nick M <4718+rkage@users.noreply.github.com>
@github-actions

github-actions Bot commented Feb 1, 2021

Copy link
Copy Markdown

Terraform Format and Style 🖌success

Terraform Initialization ⚙️success

Terraform Validation 🤖Success! The configuration is valid.

Terraform Plan 📖success

Show Plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

google_compute_network.vpc: Refreshing state... [id=projects/raspbernetes/global/networks/raspbernetes-vpc]
google_compute_subnetwork.subnet: Refreshing state... [id=projects/raspbernetes/regions/us-central1/subnetworks/raspbernetes-subnet]
google_container_cluster.primary: Refreshing state... [id=projects/raspbernetes/locations/us-central1/clusters/raspbernetes-gke]
google_container_node_pool.primary_nodes: Refreshing state... [id=projects/raspbernetes/locations/us-central1/clusters/raspbernetes-gke/nodePools/raspbernetes-gke-node-pool]

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

Pusher: @rkage, Action: pull_request, Working Directory: infrastructure/gcp, Workflow: terraform-plan

unsafeSkipCAVerification: false
caCertHashes:
- sha256:{{ cluster_ca_sha256 }}
- sha256:{{ cluster_ca_sha256 }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- sha256:{{ cluster_ca_sha256 }}
- sha256: {{ cluster_ca_sha256 }}

kind: InitConfiguration
bootstrapTokens:
- token: {{ kubeadm_join_token }}
ttl: 1h

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
ttl: 1h
ttl: 24h

Comment on lines +8 to +12
groups:
- system:bootstrappers:kubeadm:default-node-token
usages:
- signing
- authentication

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these still requried for 1.20.x?

bindPort: {{ cluster_apiserver_bind_port }}
certificateKey: {{ kubeadm_certificate_key }}
nodeRegistration:
{% if inventory_hostname in groups['masters'] and inventory_hostname not in groups['workers'] %}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How do you feel about adding labels on worker nodes with node-role.kubernetes.io/worker ?

@anthr76

anthr76 commented Feb 1, 2021

Copy link
Copy Markdown
Member

The SSH key being dropped is a known shared private key. This allows you to setup a group of vms with terraforms then provide ansible with a identity file. This allows us not having to share our personal SSH public keys on the readily available terraform.

Shown here:

- ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC2G7k0zGAjd+0LzhbPcGLkdJrJ/LbLrFxtXe+LPAkrphizfRxdZpSC7Dvr5Vewrkd/kfYObiDc6v23DHxzcilVC2HGLQUNeUer/YE1mL4lnXC1M3cb4eU+vJ/Gyr9XVOOReDRDBCwouaL7IzgYNCsm0O5v2z/w9ugnRLryUY180/oIGeE/aOI1HRh6YOsIn7R3Rv55y8CYSqsbmlHWiDC6iZICZtvYLYmUmCgPX2Fg2eT+aRbAStUcUERm8h246fs1KxywdHHI/6o3E1NNIPIQ0LdzIn5aWvTCd6D511L4rf/k5zbdw/Gql0AygHBR/wnngB5gSDERLKfigzeIlCKf Unsafe Shared Key

We can drop it if we want but it makes provisioning quick and easy. It's not unsafe to persist this in the repo as this keypair is only designed for building up local vms and is no longer used after they're torn down

@xunholy

xunholy commented Jul 25, 2021

Copy link
Copy Markdown
Member

We'll need to update this with the new control-plane/node PR to remove master/worker logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants