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

add rbac content and a candidate for more versioning information #1023

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

Bradamant3
Copy link
Contributor

@Bradamant3 Bradamant3 commented Oct 30, 2018

@Bradamant3
Copy link
Contributor Author

New commit adds draft content for issue #387 (can break content out into PRs/branches per issue, but it's getting cumbersome.)


1. Run `kubectl -n heptio-ark get svc/minio -o jsonpath='{.spec.ports[0].nodePort}'` to get the value of the NodePort address.

1. In `examples/minio/05-ark-backupstoragelocation.yaml`, change the value of `spec.config.s3Url` to the value of the NodePort address returned in the previous step.
Copy link
Contributor

Choose a reason for hiding this comment

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

The NodePort only contains a port, such as 35464. The value of the s3Url needs to be a fully valid URL, including http:// or https://. They can use the DNS name or IP address of any of the nodes in the cluster.


1. In `examples/minio/05-ark-backupstoragelocation.yaml`, change the value of `spec.config.s3Url` to the value of the NodePort address returned in the previous step.

1. You can now run `ark backup logs <optiional_backup_name>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

You have to specify the name of a backup; it is not optional.

Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Added some thoughts on the minikube/minio stuff. Happy to talk through in person

See [Set up Ark on your platform][3] for how to configure Ark for a production environment.
See [Set up Ark on your platform][3] for how to configure Ark for a production environment.

If you run Minikube for your cluster, you must also perform some additional steps. See [Minikube configuration][2]. Note that you'll need to make similar modifications to view the Minio logs.
Copy link
Contributor

Choose a reason for hiding this comment

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

So, couple things here - (1) the additional steps are not Minikube-specific; this will be necessary for anyone using this "get started" guide. The additional steps are needed because minio is running inside the cluster. . (2) since they'll always be needed, we should revise our example YAML here (make the minio service NodePort by default; update 05-ark-backupstoragelocation.yaml to have an s3Url placeholder that implies it needs to be reachable from wherever the ark client is running). Really the only minikube-specific thing here is how to get that externally-reachable s3Url.


1. In `examples/minio/00-minio-deployment.yaml`, change the value of `spec.type` in the `Service` YAML from `ClusterIP` to `NodePort`.

1. Run `minikube service minio --namespace=heptio-ark --url` to get the value of the Minio URL.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would split Set up server's Step 1 into some separate steps:

  • apply prereqs
  • apply minio deployment
  • get minio URL (this will be NODE_URL_OR_IP:NODE_PORT, where NODE_URL_OR_IP can be gotten in the way described here if running on minikube, and NODE_PORT will be generated by Kubernetes after the previous step -- we can add a kubectl command to get it)
  • update 05-ark-backupstoragelocation.yaml to set s3Url to the minio URL from the previous step
  • apply 20-ark-deployment.yaml, 30-restic-daemonset.yaml

Then the Minikube configuration section can go away entirely, since per previous comment it's not really minikube-specific AND it's always going to be required if running minio in-cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this all makes much more sense than what I had before -- it did seem to me that some of the content was not minikube specific but without testing I wasn't sure what.

I have a bit of kubectl for getting the nodeport, but if I'm understanding correctly to get the full URL not on minikube you have to concatenate values? I could use some help with the right commands/script to make this easier on users.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, so if you're not on minikube you need to get the external IP/DNS name of the node that the minio pod is running on. Not sure that we'll be able to provide any kind of generic command for that - we may just have to describe it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can help out with relevant commands, though - if you leave placeholders I can pull some things together.

Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Revisions are looking great @Bradamant3! Couple minor comments. Haven't looked at the RBAC content at all yet, will do that next.

1. Start the server:

```shell
kubectl apply -f examples/minio/20-ark-deployment.yaml -f examples/minio/30-restic-daemonset.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need/want the second -f there

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'll make a point of updating the upstream docs, then ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like I'm wrong here so ignore. For the record you can either do it like you have it, or put a comma (no space) between file names (e.g. -f examples/minio/20-ark-deployment.yaml,examples/minio/30-restic-daemonset.yaml)


- in any other environment

1. Get the value of the IP address or DNS name of any node in your cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should note that this needs to be an external IP (i.e. routable from where the ark client is being run)

@@ -63,7 +63,7 @@ metadata:
labels:
component: minio
spec:
type: ClusterIP
type: NodePort
Copy link
Contributor

Choose a reason for hiding this comment

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

@skriss do you want to make this change, or instead possibly consider #1006 combined with some form of ingress for minio?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the NodePort path is the lowest-friction one for a quickstart, while #1006 + ingress is probably more of a production-like setup...so I guess both imho?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having both documented would be ideal. Although I would recommend ClusterIP+ingress as one option, and NodePort as the other. In other words, don't make a blanked change from ClusterIP to NodePort.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine with me to document both as alternatives. We might want to have 2 alternate sets of YAML in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Bradamant3 have you seen andy's comment on this re: documenting both a nodeport and an ingress approach here?

@ncdc
Copy link
Contributor

ncdc commented Nov 5, 2018

@Bradamant3 if merging this PR will close out various issues, please update this PR's description so that each issue is listed individually as "fixes #xyz" or "closes #xyz". This has to be done once per issue - you can't write "fixes #abc #def #ghi", unfortunately.

@ncdc
Copy link
Contributor

ncdc commented Nov 5, 2018

This should also close #839

@Bradamant3
Copy link
Contributor Author

@Bradamant3 if merging this PR will close out various issues, please update this PR's description so that each issue is listed individually as "fixes #xyz" or "closes #xyz". This has to be done once per issue - you can't write "fixes #abc #def #ghi", unfortunately.

See changes in description -- is this what you mean?

@ncdc
Copy link
Contributor

ncdc commented Nov 7, 2018

@Bradamant3 perfect, thanks (you'll also notice that GitHub underlines the word "fixes" to indicate it's picking up on it).

Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Added some comments on RBAC; the main thing to address is @ncdc's comments re: documenting an additional approach for getting minio logs (which will require #1006 to merge first).

docs/rbac.md Outdated

By default Ark runs with an RBAC policy of ClusterRole `cluster-admin`. This is to make sure that Ark can back up or restore anything in your cluster. But `cluster-admin` access is wide open -- it gives Ark components access to everything in your cluster. Depending on your environment and your security needs, you should consider whether to configure more restrictive access.

More fine-grained access control should also be part of more fine-grained configuration of [namespace-based backups and restores][4].
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not exactly sure what this sentence is intending to convey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. I only sorta recall/understand what I was trying to say; I'll add it back in later if/when I get it clearer.

docs/rbac.md Outdated

More fine-grained access control should also be part of more fine-grained configuration of [namespace-based backups and restores][4].

**Note:** Roles and RoleBindings are associated with a single namespaces, not with an entire cluster. PersistentVolume backups are associated only with an entire cluster. This means that any backups or restores that use a restrictive Role and RoleBinding pair can manage only the resources that belong to the namespace. PersistentVolumes cannot be backed up with more restrictive RBAC policies.
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no reason a user couldn't set up a Role/Binding for a single namespace, and a ClusterRole/Binding that allows ark to back up any PV in the cluster. The limitation is really that it's all-or-none for each cluster-scoped resource type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, took me a few readings to see what you meant. But I think I got it in latest commit?

[25]: https://kubernetes.slack.com/messages/ark-dr
[26]: get-started.md#minikube-configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't look like 26 is used anywhere?

@@ -26,13 +26,45 @@ NOTE: Make sure to check out the appropriate version. We recommend that you chec

### Set up server

1. Start the server and the local storage service. In the root directory of Ark, run:
These instructions assume that you are running Minio inside your cluster. They should be used for a test environment or to explore Ark only. Service of type `NodePort` is not recommended for production.
Copy link
Contributor

Choose a reason for hiding this comment

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

Service of type NodePort is not recommended for production. might be worded a bit too strongly/broadly? I think it might be better to say that configuring Minio for a production-quality setup is out of scope here (e.g. you'll want to configure Minio for backups / high availability).

@@ -26,5 +26,7 @@ spec:
region: minio
s3ForcePathStyle: "true"
s3Url: http://minio.heptio-ark.svc:9000
# OR get minio URL per documentation (comment out previous line)
# s3Url: NODE_URL_OR_IP:NODE_PORT
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to include http:// or https:// - if it's just 1.2.3.4:31234, I'm pretty sure it won't work.

@@ -63,6 +63,9 @@ metadata:
labels:
component: minio
spec:
# ClusterIP is recommended for production environments
Copy link
Contributor

Choose a reason for hiding this comment

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

Add period at end of line

@@ -63,6 +63,9 @@ metadata:
labels:
component: minio
spec:
# ClusterIP is recommended for production environments
# change to NodePort if needed per documentation,
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize Change

1. Start the server:

```shell
kubectl apply -f examples/minio/20-ark-deployment.yaml examples/minio/30-restic-daemonset.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
kubectl apply -f examples/minio/20-ark-deployment.yaml examples/minio/30-restic-daemonset.yaml
kubectl apply -f examples/minio/20-ark-deployment.yaml -f examples/minio/30-restic-daemonset.yaml

docs/rbac.md Outdated
Here's a sample Role and RoleBinding pair.

```yaml
apiVersion: rbac.authorization.k8s.io/v1beta1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
apiVersion: rbac.authorization.k8s.io/v1beta1
apiVersion: rbac.authorization.k8s.io/v1

docs/rbac.md Outdated
```

```yaml
apiVersion: rbac.authorization.k8s.io/v1beta1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
apiVersion: rbac.authorization.k8s.io/v1beta1
apiVersion: rbac.authorization.k8s.io/v1

docs/rbac.md Outdated
metadata:
name: ROLEBINDING_NAME_HERE
subjects:
- kind: User
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- kind: User
- kind: ServiceAccount

docs/rbac.md Outdated
name: ROLEBINDING_NAME_HERE
subjects:
- kind: User
name: YOUR_USER_HERE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: YOUR_USER_HERE
name: YOUR_SERVICEACCOUNT_HERE


By default Ark runs with an RBAC policy of ClusterRole `cluster-admin`. This is to make sure that Ark can back up or restore anything in your cluster. But `cluster-admin` access is wide open -- it gives Ark components access to everything in your cluster. Depending on your environment and your security needs, you should consider whether to configure more restrictive access.

**Note:** Roles and RoleBindings are associated with a single namespaces, not with an entire cluster. PersistentVolume backups are associated only with an entire cluster. This means that any backups or restores that use a restrictive Role and RoleBinding pair can manage only the resources that belong to the namespace. You do not need a wide open RBAC policy to manage PersistentVolumes, however. You can configure a ClusterRole and ClusterRoleBinding that allow backups and restores only of PersistentVolumes, not of all objects in the cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you thinking of writing an example ClusterRole and ClusterRoleBinding for PVs?

Another RBAC issue if Ark isn't running as cluster-admin: it can't grant privileges it doesn't currently have. It can't create and bind roles or cluster roles beyond its abilities (https://kubernetes.io/docs/reference/access-authn-authz/rbac/#privilege-escalation-prevention-and-bootstrapping).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't planning to write another example for this release, but could do for an interim docs release before 1.0. Added a bit of text to clarify the issue in the meantime.

@skriss skriss mentioned this pull request Nov 13, 2018
10 tasks
@Bradamant3
Copy link
Contributor Author

latest commit pushed as candidate for merge but also for discussion (get-started.md)

@skriss
Copy link
Contributor

skriss commented Nov 13, 2018

@Bradamant3 I'm wondering if we should put steps 1, 3, 4 under a single "Optional, but recommended" step just after current step 2, with a note that it's not strictly necessary to do, but ark backup/restore logs and ark backup/restore describe won't work properly without it.

Then, under that step, have "Option A" (NodePort) and "Option B" (Ingress/ClusterIP). Option A will include current step 1, 3a, 4 (bullet 1), while Option B will include Step 4 (bullets 2 & 3, clarifying that the URL should be the ingress URL). And, for both options, let's assume #1006 merges, so for both of them, we should tell users to add a publicUrl value rather than changing the s3Url value.

I think this helps with keeping things simple while making it a little clearer what's going on with the nodeport/ingress/publicUrl stuff.

@skriss
Copy link
Contributor

skriss commented Nov 13, 2018

Happy to pair on this this afternoon. I think it's close and getting it right will be really great for the v0.10 release!

@Bradamant3
Copy link
Contributor Author

Pushed a variation on your approach, @skriss, with some SOMETHINGSOMTHING placeholders where I'm not sure what to say about why you'd find a particular config or approach the right choice.

@@ -26,7 +26,9 @@ NOTE: Make sure to check out the appropriate version. We recommend that you chec

### Set up server

1. Start the server and the local storage service. In the root directory of Ark, run:
These instructions set up an Ark server that lets you test only the most basic `ark create backup` and `ark create restore` commands. For the additional configuration you need to get logs or run `ark describe` commands, see the following section.
Copy link
Contributor

Choose a reason for hiding this comment

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

The first sentence reads a little strangely to me. Maybe something like "These instructions start the Ark server and a Minio instance that is accessible from within the cluster only. To be able to access logs and run ark describe commands, you need to be able to access Minio from outside the cluster. See the following section..."

@@ -46,6 +48,42 @@ NOTE: Make sure to check out the appropriate version. We recommend that you chec
kubectl get deployments --namespace=nginx-example
```

### (Optional) Edit configurations for different environments
Copy link
Contributor

Choose a reason for hiding this comment

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

The title here is pretty generic, I'd prefer something more like "expose Minio outside the cluster" (to be able to run ark logs and ark describe)

@@ -46,6 +48,42 @@ NOTE: Make sure to check out the appropriate version. We recommend that you chec
kubectl get deployments --namespace=nginx-example
```

### (Optional) Edit configurations for different environments

When you run commands to get logs or describe a backup, the Ark server generates a pre-signed URL to download the requested items. With Minio, you can encounter DNS errors if you're running the Ark client locally. To avoid these errors, you need to edit the YAML config files for the example deployment before you set up the Ark server. If you're running Minikube, you might also find it easier to change the Service type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "With Minio, you can...." I'd say something like "To be able to access these URLs from outside the cluster (i.e. from your laptop running the ark client), you need to expose Minio outside the cluster. There are two primary ways to do this: by making the Minio service a NodePort service, or by setting up an Ingress..."


When you run commands to get logs or describe a backup, the Ark server generates a pre-signed URL to download the requested items. With Minio, you can encounter DNS errors if you're running the Ark client locally. To avoid these errors, you need to edit the YAML config files for the example deployment before you set up the Ark server. If you're running Minikube, you might also find it easier to change the Service type.

#### Add a field for the DownloadRequest URL
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether the user goes with a NodePort service or Ingress, the action they will take is updating the publicUrl field in the backup storage location config. So this section is kind of weird as-is. I'd modify it to cover the case of exposing Minio via Ingress (and probably move it below the NodePort section since this is a quickstart).

Copy link
Contributor

Choose a reason for hiding this comment

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

Re: details of what we want to cover for the Ingress option, I'll defer to what you and @ncdc discussed.


- In `examples/minio/05-ark-backupstoragelocation.yaml`, specify the value of the `publicUrl` field TO WHAT?

#### If you run Ark on Minikube or if SOMETHINGSOMETHING
Copy link
Contributor

Choose a reason for hiding this comment

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

This section should be re-titled to indicate it's for exposing Minio via a NodePort service (not Minikube-specific),.


#### If you run Ark on Minikube or if SOMETHINGSOMETHING

The Minio deployment by default specifies a Service of type `ClusterIP`. You can change this to `NodePort` for easier SOMETHINGSOMETHING? OR IF YOU DON'T WANT TO BOTHER WITH INGRESS?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, NodePort is a super-easy way to expose an in-cluster service externally, though not necessarily the thing you'd do in a production scenario. Assumes that you can directly access the node from wherever you're running the ark client.

@@ -26,5 +26,7 @@ spec:
region: minio
s3ForcePathStyle: "true"
s3Url: http://minio.heptio-ark.svc:9000
# OR get minio URL (including http/s) per documentation (comment out previous line)
# s3Url: NODE_URL_OR_IP:NODE_PORT
Copy link
Contributor

Choose a reason for hiding this comment

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

let's change this line to be publicUrl: ... -- they'll leave the s3Url as-is, and only change publicUrl

- Change the Minio Service type from `ClusterIP` to `NodePort`.
- Set up Ingress for your cluster, keeping Minio Service type `ClusterIP`.

In Ark 0.10, you also specify the value of a new `publicUrl` field for the pre-signed URL in your backup storage config.
Copy link
Contributor

Choose a reason for hiding this comment

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

you can also?


#### Specify `publicUrl` for external access

Whether you expose Minio with Service of type `NodePort` or with Ingress, if you want to get logs or run other commands that require external access, in Ark 0.10 you provide the download URL to avoid errors.
Copy link
Contributor

Choose a reason for hiding this comment

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

you can provide?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can technically configure s3Url to the NodePort url without using the publicUrl.


Whether you expose Minio with Service of type `NodePort` or with Ingress, if you want to get logs or run other commands that require external access, in Ark 0.10 you provide the download URL to avoid errors.

- In `examples/minio/05-ark-backupstoragelocation.yaml`, specify the value of the `publicUrl` field TO WHAT?
Copy link
Contributor

Choose a reason for hiding this comment

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

to either the NodePort-based URL (which isn't strictly required, because both the ark server and command line tools should be able to reach the NodePort-based URL from s3Url), or the appropriate url and port associated with the ingress.


The Minio deployment by default specifies a Service of type `ClusterIP`. You can change this to `NodePort` to easily expose a cluster service externally if you can reach the node from your Ark client.

You must also get the Minio URL, which you can then specify as the value of either the `s3Url` or the new `publicUrl` field in your backup storage config.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it's confusing to say you can specify either but not explain why or what (if anything) would change, this is why I wanted to just be updating publicUrl. I'd either explain that in this case you can change either because Ark inside the cluster should be able to access the nodeport URL, or change the text to specify changing one of them (personally I'd choose publicUrl).

kubectl -n heptio-ark get svc/minio -o jsonpath='{.spec.ports[0].nodePort}'
```

1. In `examples/minio/05-ark-backupstoragelocation.yaml`, provide this Minio URL as the value of either the `s3Url` or the `publicUrl` field. You must include the `http://` or `https://` prefix.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it's confusing to say you can specify either but not explain why or what (if anything) would change, this is why I wanted to just be updating publicUrl. I'd either explain that in this case you can change either because Ark inside the cluster should be able to access the nodeport URL, or change the text to specify changing one of them (personally I'd choose publicUrl).

Previous comment applies here too


1. Keep the Service type as `ClusterIP`.

1. In In `examples/minio/05-ark-backupstoragelocation.yaml`, provide the URL and port of your Ingress as the value of the `publicUrl` field
Copy link
Contributor

Choose a reason for hiding this comment

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

. at end of sentence.

@@ -26,5 +26,6 @@ spec:
region: minio
s3ForcePathStyle: "true"
s3Url: http://minio.heptio-ark.svc:9000
publicUrl: https://minio.mycluster.com
Copy link
Contributor

Choose a reason for hiding this comment

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

I would leave this commented out by default, with a brief comment.

@skriss
Copy link
Contributor

skriss commented Nov 14, 2018

@Bradamant3 couple new comments but overall I think this is looking pretty good!

@ncdc
Copy link
Contributor

ncdc commented Nov 14, 2018

With s3, we assume the s3 endpoint is accessible from both inside the cluster and from wherever you're running the ark CLI.

I know we've had a lot of users run into the Minio accessibility issue. How important do we think it is to add this publicUrl support? Could/should we instead make it a requirement that Minio be accessible in the same manner as s3? I know I wrote the pending publicUrl code, but is this an example where we're being too accommodating? Or should we just merge it because it's helpful?

@skriss
Copy link
Contributor

skriss commented Nov 14, 2018

I think it's helpful and solves short-term pain, so I'd opt to move ahead for now. We could look at more significantly revamping our get-started guide to use something besides an in-cluster Minio, in which case we might be able to avoid the issue altogether, but IMHO we can do that later.

@ncdc
Copy link
Contributor

ncdc commented Nov 14, 2018

Works for me

…versioning information

Signed-off-by: JENNIFER RONDEAU <[email protected]>
@Bradamant3
Copy link
Contributor Author

Responded to @skriss's latest comments. We can revisit after release, too. Might ultimately be better to put all the "make external access happen with the getting started example" in its own topic/section, but not until after release ;)

Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

This LGTM and I'm ready to merge, pending any further input from @ncdc

@ncdc
Copy link
Contributor

ncdc commented Nov 14, 2018

No further comments. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants